| Message ID | 20190330141637.22632-1-boris.brezillon@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | eeprom: at25: Convert the driver to the spi-mem interface | expand |
Hi Boris, On Sat, Mar 30, 2019 at 3:16 PM Boris Brezillon <boris.brezillon@collabora.com> wrote: > The AT25 protocol fits pretty well in the spi-mem model. Convert the > at25 spi driver to a spi-mem driver and use the dirmap API instead of > forging SPI messages manually. > This makes the driver compatible with spi-mem-only controllers > (controllers implementing only the spi_mem ops). Thanks for your patch! > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/drivers/misc/eeprom/at25.c > +++ b/drivers/misc/eeprom/at25.c > @@ -63,17 +68,89 @@ struct at25_data { > > #define io_limit PAGE_SIZE /* bytes */ > > +static int at25_create_dirmaps(struct at25_data *at25) > +{ > + struct spi_mem_dirmap_info info = { > + .op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_READ, 1), > + SPI_MEM_OP_ADDR(at25->addrlen, 0, 1), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_DATA_IN(0, NULL, 1)), > + .offset = 0, > + .length = at25->chip.byte_len, > + }; > + struct device *dev = &at25->spimem->spi->dev; > + > + if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR) > + info.length = 256; Note that by hardcoding 256 you do loose the ability to support chips where EE_INSTR_BIT3_IS_ADDR is set, _and_ more than one address byte is used (i.e. extra bit is A16 or A24). While the code parsing "address-width" doesn't handle that, the comment for EE_INSTR_BIT3_IS_ADDR suggests such chips do exist (I doubt it, though, but you may know better), and the flag can be enabled through the "at25,addr-mode" property, or through platform_data. > @@ -82,38 +159,14 @@ static int at25_ee_read(void *priv, unsigned int offset, > if (unlikely(!count)) > return -EINVAL; > > - cp = command; > - > - instr = AT25_READ; > - if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR) > - if (offset >= (1U << (at25->addrlen * 8))) > - instr |= AT25_INSTR_BIT3; > - *cp++ = instr; > - > - /* 8/16/24-bit address is written MSB first */ > - switch (at25->addrlen) { > - default: /* case 3 */ > - *cp++ = offset >> 16; > - /* fall through */ > - case 2: > - *cp++ = offset >> 8; > - /* fall through */ > - case 1: > - case 0: /* can't happen: for better codegen */ > - *cp++ = offset >> 0; > + if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && offset > 255) { Loss of support for address bit A16/A24. > + desc = at25->dirmap.rdesc[1]; > + dirmap_offset = offset - 256; > + } else { > + desc = at25->dirmap.rdesc[0]; > + dirmap_offset = offset; > } > @@ -158,48 +211,37 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count) > */ > mutex_lock(&at25->lock); > do { > + struct spi_mem_dirmap_desc *desc; > unsigned long timeout, retries; > unsigned segment; > - unsigned offset = (unsigned) off; > - u8 *cp = bounce; > + unsigned int dirmap_offset; > int sr; > - u8 instr; > > - *cp = AT25_WREN; > - status = spi_write(at25->spi, cp, 1); > + status = at25_wren(at25); > if (status < 0) { > - dev_dbg(&at25->spi->dev, "WREN --> %d\n", status); > + dev_dbg(&at25->spimem->spi->dev, "WREN --> %d\n", > + status); > break; > } > > - instr = AT25_WRITE; > - if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR) > - if (offset >= (1U << (at25->addrlen * 8))) > - instr |= AT25_INSTR_BIT3; > - *cp++ = instr; > - > - /* 8/16/24-bit address is written MSB first */ > - switch (at25->addrlen) { > - default: /* case 3 */ > - *cp++ = offset >> 16; > - /* fall through */ > - case 2: > - *cp++ = offset >> 8; > - /* fall through */ > - case 1: > - case 0: /* can't happen: for better codegen */ > - *cp++ = offset >> 0; > + if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && off > 255) { Loss of support for address bit A16/A24. > + desc = at25->dirmap.wdesc[1]; > + dirmap_offset = off - 256; Two spaces after minus sign. > + } else { > + desc = at25->dirmap.wdesc[0]; > + dirmap_offset = off; > } > @@ -211,10 +253,9 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count) > timeout = jiffies + msecs_to_jiffies(EE_TIMEOUT); > retries = 0; > do { > - > - sr = spi_w8r8(at25->spi, AT25_RDSR); > + sr = at25_rdsr(at25); > if (sr < 0 || (sr & AT25_SR_nRDY)) { > - dev_dbg(&at25->spi->dev, > + dev_dbg(&at25->spimem->spi->dev, > "rdsr --> %d (%02x)\n", sr, sr); You might want to move the debug print inside the new function at25_rdsr(), as there's another call plus debug print in at25_probe(). > @@ -328,37 +369,49 @@ static int at25_probe(struct spi_device *spi) > else if (chip.flags & EE_ADDR3) > addrlen = 3; > else { > - dev_dbg(&spi->dev, "unsupported address type\n"); > + dev_dbg(&spimem->spi->dev, "unsupported address type\n"); > return -EINVAL; > } > > - /* Ping the chip ... the status register is pretty portable, > - * unlike probing manufacturer IDs. We do expect that system > - * firmware didn't write it in the past few milliseconds! > - */ > - sr = spi_w8r8(spi, AT25_RDSR); > - if (sr < 0 || sr & AT25_SR_nRDY) { > - dev_dbg(&spi->dev, "rdsr --> %d (%02x)\n", sr, sr); > - return -ENXIO; > - } > - > - at25 = devm_kzalloc(&spi->dev, sizeof(struct at25_data), GFP_KERNEL); > + at25 = devm_kzalloc(&spimem->spi->dev, sizeof(struct at25_data), > + GFP_KERNEL); > if (!at25) > return -ENOMEM; > > mutex_init(&at25->lock); > at25->chip = chip; > - at25->spi = spi; > - spi_set_drvdata(spi, at25); > + at25->spimem = spimem; > + spi_mem_set_drvdata(spimem, at25); > at25->addrlen = addrlen; > > - at25->nvmem_config.name = dev_name(&spi->dev); > - at25->nvmem_config.dev = &spi->dev; > + /* > + * Can't be allocated with devm_kmalloc() because we need a DMA-safe > + * buffer. > + */ That is no longer true as of commit a66d972465d15b1d ("devres: Align data[] to ARCH_KMALLOC_MINALIGN") in v4.20-rc5, right? > + at25->scratchbuf = kmalloc(1, GFP_KERNEL); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Boris,
I love your patch! Yet something to improve:
[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v5.1-rc3 next-20190401]
[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/Boris-Brezillon/eeprom-at25-Convert-the-driver-to-the-spi-mem-interface/20190401-160450
config: i386-randconfig-m2-201913 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
>> ERROR: "spi_mem_driver_unregister" [drivers/misc/eeprom/at25.ko] undefined!
>> ERROR: "spi_mem_driver_register_with_owner" [drivers/misc/eeprom/at25.ko] undefined!
>> ERROR: "devm_spi_mem_dirmap_create" [drivers/misc/eeprom/at25.ko] undefined!
>> ERROR: "spi_mem_dirmap_write" [drivers/misc/eeprom/at25.ko] undefined!
>> ERROR: "spi_mem_dirmap_read" [drivers/misc/eeprom/at25.ko] undefined!
>> ERROR: "spi_mem_exec_op" [drivers/misc/eeprom/at25.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Boris,
I love your patch! Yet something to improve:
[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v5.1-rc3 next-20190401]
[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/Boris-Brezillon/eeprom-at25-Convert-the-driver-to-the-spi-mem-interface/20190401-160450
config: x86_64-randconfig-m3-201913 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/misc/eeprom/at25.o: In function `at25_rdsr':
>> drivers/misc/eeprom/at25.c:130: undefined reference to `spi_mem_exec_op'
drivers/misc/eeprom/at25.o: In function `at25_ee_read':
>> drivers/misc/eeprom/at25.c:178: undefined reference to `spi_mem_dirmap_read'
drivers/misc/eeprom/at25.o: In function `at25_create_dirmaps':
>> drivers/misc/eeprom/at25.c:86: undefined reference to `devm_spi_mem_dirmap_create'
drivers/misc/eeprom/at25.c:94: undefined reference to `devm_spi_mem_dirmap_create'
drivers/misc/eeprom/at25.c:106: undefined reference to `devm_spi_mem_dirmap_create'
drivers/misc/eeprom/at25.c:114: undefined reference to `devm_spi_mem_dirmap_create'
drivers/misc/eeprom/at25.o: In function `at25_wren':
drivers/misc/eeprom/at25.c:144: undefined reference to `spi_mem_exec_op'
drivers/misc/eeprom/at25.o: In function `at25_ee_write':
>> drivers/misc/eeprom/at25.c:240: undefined reference to `spi_mem_dirmap_write'
drivers/misc/eeprom/at25.o: In function `at25_driver_init':
>> drivers/misc/eeprom/at25.c:468: undefined reference to `spi_mem_driver_register_with_owner'
drivers/misc/eeprom/at25.o: In function `at25_driver_exit':
>> drivers/misc/eeprom/at25.c:468: undefined reference to `spi_mem_driver_unregister'
vim +130 drivers/misc/eeprom/at25.c
70
71 static int at25_create_dirmaps(struct at25_data *at25)
72 {
73 struct spi_mem_dirmap_info info = {
74 .op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_READ, 1),
75 SPI_MEM_OP_ADDR(at25->addrlen, 0, 1),
76 SPI_MEM_OP_NO_DUMMY,
77 SPI_MEM_OP_DATA_IN(0, NULL, 1)),
78 .offset = 0,
79 .length = at25->chip.byte_len,
80 };
81 struct device *dev = &at25->spimem->spi->dev;
82
83 if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
84 info.length = 256;
85
> 86 at25->dirmap.rdesc[0] = devm_spi_mem_dirmap_create(dev, at25->spimem,
87 &info);
88 if (IS_ERR(at25->dirmap.rdesc[0]))
89 return PTR_ERR(at25->dirmap.rdesc[0]);
90
91 info.op_tmpl.cmd.opcode = AT25_WRITE;
92 info.op_tmpl.data.dir = SPI_MEM_DATA_OUT;
93
94 at25->dirmap.wdesc[0] = devm_spi_mem_dirmap_create(dev, at25->spimem,
95 &info);
96 if (IS_ERR(at25->dirmap.wdesc[0]))
97 return PTR_ERR(at25->dirmap.wdesc[0]);
98
99 if (!(at25->chip.flags & EE_INSTR_BIT3_IS_ADDR))
100 return 0;
101
102 info.length = at25->chip.byte_len - 256;
103 info.op_tmpl.cmd.opcode = AT25_READ | AT25_INSTR_BIT3;
104 info.op_tmpl.data.dir = SPI_MEM_DATA_IN;
105
> 106 at25->dirmap.rdesc[1] = devm_spi_mem_dirmap_create(dev, at25->spimem,
107 &info);
108 if (IS_ERR(at25->dirmap.rdesc[1]))
109 return PTR_ERR(at25->dirmap.rdesc[1]);
110
111 info.op_tmpl.cmd.opcode = AT25_WRITE | AT25_INSTR_BIT3;
112 info.op_tmpl.data.dir = SPI_MEM_DATA_OUT;
113
114 at25->dirmap.wdesc[1] = devm_spi_mem_dirmap_create(dev, at25->spimem,
115 &info);
116 if (IS_ERR(at25->dirmap.wdesc[1]))
117 return PTR_ERR(at25->dirmap.wdesc[1]);
118
119 return 0;
120 }
121
122 static int at25_rdsr(struct at25_data *at25)
123 {
124 struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_RDSR, 1),
125 SPI_MEM_OP_NO_ADDR,
126 SPI_MEM_OP_NO_DUMMY,
127 SPI_MEM_OP_DATA_IN(1, at25->scratchbuf, 1));
128 int ret;
129
> 130 ret = spi_mem_exec_op(at25->spimem, &op);
131 if (ret)
132 return ret;
133
134 return *((u8 *)at25->scratchbuf);
135 }
136
137 static int at25_wren(struct at25_data *at25)
138 {
139 struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_WREN, 1),
140 SPI_MEM_OP_NO_ADDR,
141 SPI_MEM_OP_NO_DUMMY,
142 SPI_MEM_OP_NO_DATA);
143
144 return spi_mem_exec_op(at25->spimem, &op);
145 }
146
147 static int at25_ee_read(void *priv, unsigned int offset,
148 void *val, size_t count)
149 {
150 struct spi_mem_dirmap_desc *desc;
151 struct at25_data *at25 = priv;
152 unsigned int dirmap_offset;
153 ssize_t status;
154
155 if (unlikely(offset >= at25->chip.byte_len))
156 return -EINVAL;
157 if ((offset + count) > at25->chip.byte_len)
158 count = at25->chip.byte_len - offset;
159 if (unlikely(!count))
160 return -EINVAL;
161
162 if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && offset > 255) {
163 desc = at25->dirmap.rdesc[1];
164 dirmap_offset = offset - 256;
165 } else {
166 desc = at25->dirmap.rdesc[0];
167 dirmap_offset = offset;
168 }
169
170 mutex_lock(&at25->lock);
171
172 /* Read it all at once.
173 *
174 * REVISIT that's potentially a problem with large chips, if
175 * other devices on the bus need to be accessed regularly or
176 * this chip is clocked very slowly
177 */
> 178 status = spi_mem_dirmap_read(desc, dirmap_offset, count, val);
179 dev_dbg(&at25->spimem->spi->dev, "read %zu bytes at %d --> %zd\n",
180 count, offset, status);
181
182 mutex_unlock(&at25->lock);
183 return status;
184 }
185
186 static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
187 {
188 struct at25_data *at25 = priv;
189 const char *buf = val;
190 int status = 0;
191 unsigned buf_size;
192 u8 *bounce;
193
194 if (unlikely(off >= at25->chip.byte_len))
195 return -EFBIG;
196 if ((off + count) > at25->chip.byte_len)
197 count = at25->chip.byte_len - off;
198 if (unlikely(!count))
199 return -EINVAL;
200
201 /* Temp buffer starts with command and address */
202 buf_size = at25->chip.page_size;
203 if (buf_size > io_limit)
204 buf_size = io_limit;
205 bounce = kmalloc(buf_size, GFP_KERNEL);
206 if (!bounce)
207 return -ENOMEM;
208
209 /* For write, rollover is within the page ... so we write at
210 * most one page, then manually roll over to the next page.
211 */
212 mutex_lock(&at25->lock);
213 do {
214 struct spi_mem_dirmap_desc *desc;
215 unsigned long timeout, retries;
216 unsigned segment;
217 unsigned int dirmap_offset;
218 int sr;
219
220 status = at25_wren(at25);
221 if (status < 0) {
222 dev_dbg(&at25->spimem->spi->dev, "WREN --> %d\n",
223 status);
224 break;
225 }
226
227 if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && off > 255) {
228 desc = at25->dirmap.wdesc[1];
229 dirmap_offset = off - 256;
230 } else {
231 desc = at25->dirmap.wdesc[0];
232 dirmap_offset = off;
233 }
234
235 /* Write as much of a page as we can */
236 segment = buf_size - (dirmap_offset % buf_size);
237 if (segment > count)
238 segment = count;
239 memcpy(bounce, buf, segment);
> 240 status = spi_mem_dirmap_write(desc, dirmap_offset, segment,
241 bounce);
242 dev_dbg(&at25->spimem->spi->dev,
243 "write %u bytes at %u --> %d\n",
244 segment, off, status);
245 if (status < 0)
246 break;
247
248 /* REVISIT this should detect (or prevent) failed writes
249 * to readonly sections of the EEPROM...
250 */
251
252 /* Wait for non-busy status */
253 timeout = jiffies + msecs_to_jiffies(EE_TIMEOUT);
254 retries = 0;
255 do {
256 sr = at25_rdsr(at25);
257 if (sr < 0 || (sr & AT25_SR_nRDY)) {
258 dev_dbg(&at25->spimem->spi->dev,
259 "rdsr --> %d (%02x)\n", sr, sr);
260 /* at HZ=100, this is sloooow */
261 msleep(1);
262 continue;
263 }
264 if (!(sr & AT25_SR_nRDY))
265 break;
266 } while (retries++ < 3 || time_before_eq(jiffies, timeout));
267
268 if ((sr < 0) || (sr & AT25_SR_nRDY)) {
269 dev_err(&at25->spimem->spi->dev,
270 "write %u bytes offset %u, timeout after %u msecs\n",
271 segment, off,
272 jiffies_to_msecs(jiffies -
273 (timeout - EE_TIMEOUT)));
274 status = -ETIMEDOUT;
275 break;
276 }
277
278 off += segment;
279 buf += segment;
280 count -= segment;
281
282 } while (count > 0);
283
284 mutex_unlock(&at25->lock);
285
286 kfree(bounce);
287 return status;
288 }
289
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Sat, Mar 30, 2019 at 03:16:37PM +0100, Boris Brezillon wrote: > The AT25 protocol fits pretty well in the spi-mem model. Convert the > at25 spi driver to a spi-mem driver and use the dirmap API instead of > forging SPI messages manually. > This makes the driver compatible with spi-mem-only controllers > (controllers implementing only the spi_mem ops). > > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/misc/eeprom/at25.c | 282 +++++++++++++++++++++++-------------- > 1 file changed, 176 insertions(+), 106 deletions(-) Will there be a new version of this to fix up the problems that 0-day found in it? thanks, greg k-h
On Thu, Apr 25, 2019 at 9:44 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Sat, Mar 30, 2019 at 03:16:37PM +0100, Boris Brezillon wrote: > > The AT25 protocol fits pretty well in the spi-mem model. Convert the > > at25 spi driver to a spi-mem driver and use the dirmap API instead of > > forging SPI messages manually. > > This makes the driver compatible with spi-mem-only controllers > > (controllers implementing only the spi_mem ops). > > > > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > > --- > > drivers/misc/eeprom/at25.c | 282 +++++++++++++++++++++++-------------- > > 1 file changed, 176 insertions(+), 106 deletions(-) > > Will there be a new version of this to fix up the problems that 0-day > found in it? gmail-whitespace-damaged fix: diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig index f2abe27010eff133..98145d7d43d0c728 100644 --- a/drivers/misc/eeprom/Kconfig +++ b/drivers/misc/eeprom/Kconfig @@ -36,6 +36,7 @@ config EEPROM_AT25 depends on SPI && SYSFS select NVMEM select NVMEM_SYSFS + select SPI_MEM help Enable this driver to get read/write support to most SPI EEPROMs, after you configure the board init code to know about each eeprom Boris: what's the plan? Gr{oetje,eeting}s, Geert
Hi Geert, On Tue, 3 Sep 2019 17:06:52 +0200 Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Thu, Apr 25, 2019 at 9:44 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > On Sat, Mar 30, 2019 at 03:16:37PM +0100, Boris Brezillon wrote: > > > The AT25 protocol fits pretty well in the spi-mem model. Convert the > > > at25 spi driver to a spi-mem driver and use the dirmap API instead of > > > forging SPI messages manually. > > > This makes the driver compatible with spi-mem-only controllers > > > (controllers implementing only the spi_mem ops). > > > > > > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > > > --- > > > drivers/misc/eeprom/at25.c | 282 +++++++++++++++++++++++-------------- > > > 1 file changed, 176 insertions(+), 106 deletions(-) > > > > Will there be a new version of this to fix up the problems that 0-day > > found in it? > > gmail-whitespace-damaged fix: > > diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig > index f2abe27010eff133..98145d7d43d0c728 100644 > --- a/drivers/misc/eeprom/Kconfig > +++ b/drivers/misc/eeprom/Kconfig > @@ -36,6 +36,7 @@ config EEPROM_AT25 > depends on SPI && SYSFS > select NVMEM > select NVMEM_SYSFS > + select SPI_MEM > help > Enable this driver to get read/write support to most SPI EEPROMs, > after you configure the board init code to know about each eeprom > > Boris: what's the plan? Sorry, I don't have time to respin this patch. Feel free to send a new version if you need it. Regards, Boris
diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c index 99de6939cd5a..818853babbd0 100644 --- a/drivers/misc/eeprom/at25.c +++ b/drivers/misc/eeprom/at25.c @@ -17,7 +17,7 @@ #include <linux/sched.h> #include <linux/nvmem-provider.h> -#include <linux/spi/spi.h> +#include <linux/spi/spi-mem.h> #include <linux/spi/eeprom.h> #include <linux/property.h> @@ -29,12 +29,17 @@ */ struct at25_data { - struct spi_device *spi; + struct spi_mem *spimem; struct mutex lock; struct spi_eeprom chip; unsigned addrlen; struct nvmem_config nvmem_config; struct nvmem_device *nvmem; + void *scratchbuf; + struct { + struct spi_mem_dirmap_desc *rdesc[2]; + struct spi_mem_dirmap_desc *wdesc[2]; + } dirmap; }; #define AT25_WREN 0x06 /* latch the write enable */ @@ -63,17 +68,89 @@ struct at25_data { #define io_limit PAGE_SIZE /* bytes */ +static int at25_create_dirmaps(struct at25_data *at25) +{ + struct spi_mem_dirmap_info info = { + .op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_READ, 1), + SPI_MEM_OP_ADDR(at25->addrlen, 0, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_IN(0, NULL, 1)), + .offset = 0, + .length = at25->chip.byte_len, + }; + struct device *dev = &at25->spimem->spi->dev; + + if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR) + info.length = 256; + + at25->dirmap.rdesc[0] = devm_spi_mem_dirmap_create(dev, at25->spimem, + &info); + if (IS_ERR(at25->dirmap.rdesc[0])) + return PTR_ERR(at25->dirmap.rdesc[0]); + + info.op_tmpl.cmd.opcode = AT25_WRITE; + info.op_tmpl.data.dir = SPI_MEM_DATA_OUT; + + at25->dirmap.wdesc[0] = devm_spi_mem_dirmap_create(dev, at25->spimem, + &info); + if (IS_ERR(at25->dirmap.wdesc[0])) + return PTR_ERR(at25->dirmap.wdesc[0]); + + if (!(at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)) + return 0; + + info.length = at25->chip.byte_len - 256; + info.op_tmpl.cmd.opcode = AT25_READ | AT25_INSTR_BIT3; + info.op_tmpl.data.dir = SPI_MEM_DATA_IN; + + at25->dirmap.rdesc[1] = devm_spi_mem_dirmap_create(dev, at25->spimem, + &info); + if (IS_ERR(at25->dirmap.rdesc[1])) + return PTR_ERR(at25->dirmap.rdesc[1]); + + info.op_tmpl.cmd.opcode = AT25_WRITE | AT25_INSTR_BIT3; + info.op_tmpl.data.dir = SPI_MEM_DATA_OUT; + + at25->dirmap.wdesc[1] = devm_spi_mem_dirmap_create(dev, at25->spimem, + &info); + if (IS_ERR(at25->dirmap.wdesc[1])) + return PTR_ERR(at25->dirmap.wdesc[1]); + + return 0; +} + +static int at25_rdsr(struct at25_data *at25) +{ + struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_RDSR, 1), + SPI_MEM_OP_NO_ADDR, + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_IN(1, at25->scratchbuf, 1)); + int ret; + + ret = spi_mem_exec_op(at25->spimem, &op); + if (ret) + return ret; + + return *((u8 *)at25->scratchbuf); +} + +static int at25_wren(struct at25_data *at25) +{ + struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(AT25_WREN, 1), + SPI_MEM_OP_NO_ADDR, + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_NO_DATA); + + return spi_mem_exec_op(at25->spimem, &op); +} + static int at25_ee_read(void *priv, unsigned int offset, void *val, size_t count) { + struct spi_mem_dirmap_desc *desc; struct at25_data *at25 = priv; - char *buf = val; - u8 command[EE_MAXADDRLEN + 1]; - u8 *cp; + unsigned int dirmap_offset; ssize_t status; - struct spi_transfer t[2]; - struct spi_message m; - u8 instr; if (unlikely(offset >= at25->chip.byte_len)) return -EINVAL; @@ -82,38 +159,14 @@ static int at25_ee_read(void *priv, unsigned int offset, if (unlikely(!count)) return -EINVAL; - cp = command; - - instr = AT25_READ; - if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR) - if (offset >= (1U << (at25->addrlen * 8))) - instr |= AT25_INSTR_BIT3; - *cp++ = instr; - - /* 8/16/24-bit address is written MSB first */ - switch (at25->addrlen) { - default: /* case 3 */ - *cp++ = offset >> 16; - /* fall through */ - case 2: - *cp++ = offset >> 8; - /* fall through */ - case 1: - case 0: /* can't happen: for better codegen */ - *cp++ = offset >> 0; + if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && offset > 255) { + desc = at25->dirmap.rdesc[1]; + dirmap_offset = offset - 256; + } else { + desc = at25->dirmap.rdesc[0]; + dirmap_offset = offset; } - spi_message_init(&m); - memset(t, 0, sizeof(t)); - - t[0].tx_buf = command; - t[0].len = at25->addrlen + 1; - spi_message_add_tail(&t[0], &m); - - t[1].rx_buf = buf; - t[1].len = count; - spi_message_add_tail(&t[1], &m); - mutex_lock(&at25->lock); /* Read it all at once. @@ -122,8 +175,8 @@ static int at25_ee_read(void *priv, unsigned int offset, * other devices on the bus need to be accessed regularly or * this chip is clocked very slowly */ - status = spi_sync(at25->spi, &m); - dev_dbg(&at25->spi->dev, "read %zu bytes at %d --> %zd\n", + status = spi_mem_dirmap_read(desc, dirmap_offset, count, val); + dev_dbg(&at25->spimem->spi->dev, "read %zu bytes at %d --> %zd\n", count, offset, status); mutex_unlock(&at25->lock); @@ -149,7 +202,7 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count) buf_size = at25->chip.page_size; if (buf_size > io_limit) buf_size = io_limit; - bounce = kmalloc(buf_size + at25->addrlen + 1, GFP_KERNEL); + bounce = kmalloc(buf_size, GFP_KERNEL); if (!bounce) return -ENOMEM; @@ -158,48 +211,37 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count) */ mutex_lock(&at25->lock); do { + struct spi_mem_dirmap_desc *desc; unsigned long timeout, retries; unsigned segment; - unsigned offset = (unsigned) off; - u8 *cp = bounce; + unsigned int dirmap_offset; int sr; - u8 instr; - *cp = AT25_WREN; - status = spi_write(at25->spi, cp, 1); + status = at25_wren(at25); if (status < 0) { - dev_dbg(&at25->spi->dev, "WREN --> %d\n", status); + dev_dbg(&at25->spimem->spi->dev, "WREN --> %d\n", + status); break; } - instr = AT25_WRITE; - if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR) - if (offset >= (1U << (at25->addrlen * 8))) - instr |= AT25_INSTR_BIT3; - *cp++ = instr; - - /* 8/16/24-bit address is written MSB first */ - switch (at25->addrlen) { - default: /* case 3 */ - *cp++ = offset >> 16; - /* fall through */ - case 2: - *cp++ = offset >> 8; - /* fall through */ - case 1: - case 0: /* can't happen: for better codegen */ - *cp++ = offset >> 0; + if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && off > 255) { + desc = at25->dirmap.wdesc[1]; + dirmap_offset = off - 256; + } else { + desc = at25->dirmap.wdesc[0]; + dirmap_offset = off; } /* Write as much of a page as we can */ - segment = buf_size - (offset % buf_size); + segment = buf_size - (dirmap_offset % buf_size); if (segment > count) segment = count; - memcpy(cp, buf, segment); - status = spi_write(at25->spi, bounce, - segment + at25->addrlen + 1); - dev_dbg(&at25->spi->dev, "write %u bytes at %u --> %d\n", - segment, offset, status); + memcpy(bounce, buf, segment); + status = spi_mem_dirmap_write(desc, dirmap_offset, segment, + bounce); + dev_dbg(&at25->spimem->spi->dev, + "write %u bytes at %u --> %d\n", + segment, off, status); if (status < 0) break; @@ -211,10 +253,9 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count) timeout = jiffies + msecs_to_jiffies(EE_TIMEOUT); retries = 0; do { - - sr = spi_w8r8(at25->spi, AT25_RDSR); + sr = at25_rdsr(at25); if (sr < 0 || (sr & AT25_SR_nRDY)) { - dev_dbg(&at25->spi->dev, + dev_dbg(&at25->spimem->spi->dev, "rdsr --> %d (%02x)\n", sr, sr); /* at HZ=100, this is sloooow */ msleep(1); @@ -225,9 +266,9 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count) } while (retries++ < 3 || time_before_eq(jiffies, timeout)); if ((sr < 0) || (sr & AT25_SR_nRDY)) { - dev_err(&at25->spi->dev, + dev_err(&at25->spimem->spi->dev, "write %u bytes offset %u, timeout after %u msecs\n", - segment, offset, + segment, off, jiffies_to_msecs(jiffies - (timeout - EE_TIMEOUT))); status = -ETIMEDOUT; @@ -304,7 +345,7 @@ static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip) return 0; } -static int at25_probe(struct spi_device *spi) +static int at25_probe(struct spi_mem *spimem) { struct at25_data *at25 = NULL; struct spi_eeprom chip; @@ -313,12 +354,12 @@ static int at25_probe(struct spi_device *spi) int addrlen; /* Chip description */ - if (!spi->dev.platform_data) { - err = at25_fw_to_chip(&spi->dev, &chip); + if (!spimem->spi->dev.platform_data) { + err = at25_fw_to_chip(&spimem->spi->dev, &chip); if (err) return err; } else - chip = *(struct spi_eeprom *)spi->dev.platform_data; + chip = *(struct spi_eeprom *)spimem->spi->dev.platform_data; /* For now we only support 8/16/24 bit addressing */ if (chip.flags & EE_ADDR1) @@ -328,37 +369,49 @@ static int at25_probe(struct spi_device *spi) else if (chip.flags & EE_ADDR3) addrlen = 3; else { - dev_dbg(&spi->dev, "unsupported address type\n"); + dev_dbg(&spimem->spi->dev, "unsupported address type\n"); return -EINVAL; } - /* Ping the chip ... the status register is pretty portable, - * unlike probing manufacturer IDs. We do expect that system - * firmware didn't write it in the past few milliseconds! - */ - sr = spi_w8r8(spi, AT25_RDSR); - if (sr < 0 || sr & AT25_SR_nRDY) { - dev_dbg(&spi->dev, "rdsr --> %d (%02x)\n", sr, sr); - return -ENXIO; - } - - at25 = devm_kzalloc(&spi->dev, sizeof(struct at25_data), GFP_KERNEL); + at25 = devm_kzalloc(&spimem->spi->dev, sizeof(struct at25_data), + GFP_KERNEL); if (!at25) return -ENOMEM; mutex_init(&at25->lock); at25->chip = chip; - at25->spi = spi; - spi_set_drvdata(spi, at25); + at25->spimem = spimem; + spi_mem_set_drvdata(spimem, at25); at25->addrlen = addrlen; - at25->nvmem_config.name = dev_name(&spi->dev); - at25->nvmem_config.dev = &spi->dev; + /* + * Can't be allocated with devm_kmalloc() because we need a DMA-safe + * buffer. + */ + at25->scratchbuf = kmalloc(1, GFP_KERNEL); + + /* Ping the chip ... the status register is pretty portable, + * unlike probing manufacturer IDs. We do expect that system + * firmware didn't write it in the past few milliseconds! + */ + sr = at25_rdsr(at25); + if (sr < 0 || sr & AT25_SR_nRDY) { + dev_dbg(&spimem->spi->dev, "rdsr --> %d (%02x)\n", sr, sr); + err = -ENXIO; + goto err_free_scratchbuf; + } + + err = at25_create_dirmaps(at25); + if (err) + goto err_free_scratchbuf; + + at25->nvmem_config.name = dev_name(&spimem->spi->dev); + at25->nvmem_config.dev = &spimem->spi->dev; at25->nvmem_config.read_only = chip.flags & EE_READONLY; at25->nvmem_config.root_only = true; at25->nvmem_config.owner = THIS_MODULE; at25->nvmem_config.compat = true; - at25->nvmem_config.base_dev = &spi->dev; + at25->nvmem_config.base_dev = &spimem->spi->dev; at25->nvmem_config.reg_read = at25_ee_read; at25->nvmem_config.reg_write = at25_ee_write; at25->nvmem_config.priv = at25; @@ -366,17 +419,34 @@ static int at25_probe(struct spi_device *spi) at25->nvmem_config.word_size = 1; at25->nvmem_config.size = chip.byte_len; - at25->nvmem = devm_nvmem_register(&spi->dev, &at25->nvmem_config); - if (IS_ERR(at25->nvmem)) - return PTR_ERR(at25->nvmem); + at25->nvmem = devm_nvmem_register(&spimem->spi->dev, + &at25->nvmem_config); + if (IS_ERR(at25->nvmem)) { + err = PTR_ERR(at25->nvmem); + goto err_free_scratchbuf; + } - dev_info(&spi->dev, "%d %s %s eeprom%s, pagesize %u\n", + dev_info(&spimem->spi->dev, "%d %s %s eeprom%s, pagesize %u\n", (chip.byte_len < 1024) ? chip.byte_len : (chip.byte_len / 1024), (chip.byte_len < 1024) ? "Byte" : "KByte", at25->chip.name, (chip.flags & EE_READONLY) ? " (readonly)" : "", at25->chip.page_size); return 0; + +err_free_scratchbuf: + kfree(at25->scratchbuf); + + return err; +} + +static int at25_remove(struct spi_mem *spimem) +{ + struct at25_data *at25 = spi_mem_get_drvdata(spimem); + + kfree(at25->scratchbuf); + + return 0; } /*-------------------------------------------------------------------------*/ @@ -387,15 +457,15 @@ static const struct of_device_id at25_of_match[] = { }; MODULE_DEVICE_TABLE(of, at25_of_match); -static struct spi_driver at25_driver = { - .driver = { - .name = "at25", +static struct spi_mem_driver at25_driver = { + .spidrv.driver = { + .name = "at25", .of_match_table = at25_of_match, }, - .probe = at25_probe, + .probe = at25_probe, + .remove = at25_remove, }; - -module_spi_driver(at25_driver); +module_spi_mem_driver(at25_driver); MODULE_DESCRIPTION("Driver for most SPI EEPROMs"); MODULE_AUTHOR("David Brownell");
The AT25 protocol fits pretty well in the spi-mem model. Convert the at25 spi driver to a spi-mem driver and use the dirmap API instead of forging SPI messages manually. This makes the driver compatible with spi-mem-only controllers (controllers implementing only the spi_mem ops). Cc: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> --- drivers/misc/eeprom/at25.c | 282 +++++++++++++++++++++++-------------- 1 file changed, 176 insertions(+), 106 deletions(-)