| Message ID | 1446824889-16144-3-git-send-email-bayi.cheng@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Bayi,
[auto build test ERROR on mtd/master]
[also build test ERROR on v4.3 next-20151106]
url: https://github.com/0day-ci/linux/commits/Bayi-Cheng/Mediatek-SPI-NOR-flash-driver/20151106-235157
base: git://git.infradead.org/linux-mtd.git master
config: i386-allmodconfig (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/mtd/spi-nor/mtk-quadspi.c: In function 'mtk_nor_init':
>> drivers/mtd/spi-nor/mtk-quadspi.c:381:5: error: 'struct spi_nor' has no member named 'flash_node'
nor->flash_node = flash_node;
^
drivers/mtd/spi-nor/mtk-quadspi.c:387:17: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
nor->write_reg = mt8173_nor_write_reg;
^
>> drivers/mtd/spi-nor/mtk-quadspi.c:388:10: error: request for member 'owner' in something not a structure or union
nor->mtd.owner = THIS_MODULE;
^
>> drivers/mtd/spi-nor/mtk-quadspi.c:389:10: error: request for member 'name' in something not a structure or union
nor->mtd.name = "mtk_nor";
^
drivers/mtd/spi-nor/mtk-quadspi.c:395:35: warning: passing argument 1 of 'mtd_device_parse_register' from incompatible pointer type [-Wincompatible-pointer-types]
return mtd_device_parse_register(&nor->mtd, NULL, &ppdata, NULL, 0);
^
In file included from drivers/mtd/spi-nor/mtk-quadspi.c:24:0:
include/linux/mtd/mtd.h:372:12: note: expected 'struct mtd_info *' but argument is of type 'struct mtd_info **'
extern int mtd_device_parse_register(struct mtd_info *mtd,
^
coccinelle warnings: (new ones prefixed by >>)
>> drivers/mtd/spi-nor/mtk-quadspi.c:223:6-28: Move constant to right.
drivers/mtd/spi-nor/mtk-quadspi.c:214:6-10: Move constant to right.
Please review and possibly fold the followup patch.
vim +381 drivers/mtd/spi-nor/mtk-quadspi.c
217 static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
218 {
219 u8 reg;
220
221 writel(MTK_NOR_WR_BUF_DISABLE, mt8173_nor->base + MTK_NOR_CFG2_REG);
222 return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg,
> 223 MTK_NOR_WR_BUF_DISABLE == (reg & 0x1), 100,
224 10000);
225 }
226
227 static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
228 {
229 int i;
230
231 for (i = 0; i < 3; i++) {
232 writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
233 addr >>= 8;
234 }
235 /* Last register is non-contiguous */
236 writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR3_REG);
237 }
238
239 static int mt8173_nor_read(struct spi_nor *nor, loff_t from, size_t length,
240 size_t *retlen, u_char *buffer)
241 {
242 int i, ret;
243 int addr = (int)from;
244 u8 *buf = (u8 *)buffer;
245 struct mt8173_nor *mt8173_nor = nor->priv;
246
247 /* set mode for fast read mode ,dual mode or quad mode */
248 mt8173_nor_set_read_mode(mt8173_nor);
249 mt8173_nor_set_addr(mt8173_nor, addr);
250
251 for (i = 0; i < length; i++, (*retlen)++) {
252 ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_READ_CMD);
253 if (ret < 0)
254 return ret;
255 buf[i] = readb(mt8173_nor->base + MTK_NOR_RDATA_REG);
256 }
257 return 0;
258 }
259
260 static int mt8173_nor_write_single_byte(struct mt8173_nor *mt8173_nor,
261 int addr, int length, u8 *data)
262 {
263 int i, ret;
264
265 mt8173_nor_set_addr(mt8173_nor, addr);
266
267 for (i = 0; i < length; i++) {
268 ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_WR_CMD);
269 if (ret < 0)
270 return ret;
271 writeb(*data++, mt8173_nor->base + MTK_NOR_WDATA_REG);
272 }
273 return 0;
274 }
275
276 static int mt8173_nor_write_buffer(struct mt8173_nor *mt8173_nor, int addr,
277 const u8 *buf)
278 {
279 int i, bufidx, data;
280
281 mt8173_nor_set_addr(mt8173_nor, addr);
282
283 bufidx = 0;
284 for (i = 0; i < SFLASH_WRBUF_SIZE; i += 4) {
285 data = buf[bufidx + 3]<<24 | buf[bufidx + 2]<<16 |
286 buf[bufidx + 1]<<8 | buf[bufidx];
287 bufidx += 4;
288 writel(data, mt8173_nor->base + MTK_NOR_PP_DATA_REG);
289 }
290 return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_WR_CMD);
291 }
292
293 static void mt8173_nor_write(struct spi_nor *nor, loff_t to, size_t len,
294 size_t *retlen, const u_char *buf)
295 {
296 int ret;
297 struct mt8173_nor *mt8173_nor = nor->priv;
298
299 ret = mt8173_nor_write_buffer_enable(mt8173_nor);
300 if (ret < 0)
301 dev_warn(mt8173_nor->dev, "write buffer enable failed!\n");
302
303 while (len >= SFLASH_WRBUF_SIZE) {
304 ret = mt8173_nor_write_buffer(mt8173_nor, to, buf);
305 if (ret < 0)
306 dev_err(mt8173_nor->dev, "write buffer failed!\n");
307 len -= SFLASH_WRBUF_SIZE;
308 to += SFLASH_WRBUF_SIZE;
309 buf += SFLASH_WRBUF_SIZE;
310 (*retlen) += SFLASH_WRBUF_SIZE;
311 }
312 ret = mt8173_nor_write_buffer_disable(mt8173_nor);
313 if (ret < 0)
314 dev_warn(mt8173_nor->dev, "write buffer disable failed!\n");
315
316 if (len) {
317 ret = mt8173_nor_write_single_byte(mt8173_nor, to, (int)len,
318 (u8 *)buf);
319 if (ret < 0)
320 dev_err(mt8173_nor->dev, "write single byte failed!\n");
321 (*retlen) += len;
322 }
323 }
324
325 static int mt8173_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
326 {
327 int ret;
328 struct mt8173_nor *mt8173_nor = nor->priv;
329
330 switch (opcode) {
331 case SPINOR_OP_RDSR:
332 ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_RDSR_CMD);
333 if (ret < 0)
334 return ret;
335 if (len == 1)
336 *buf = readb(mt8173_nor->base + MTK_NOR_RDSR_REG);
337 else
338 dev_err(mt8173_nor->dev, "len should be 1 for read status!\n");
339 break;
340 default:
341 ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, NULL, 0, buf, len);
342 break;
343 }
344 return ret;
345 }
346
347 static int mt8173_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
348 int len)
349 {
350 int ret;
351 struct mt8173_nor *mt8173_nor = nor->priv;
352
353 switch (opcode) {
354 case SPINOR_OP_WRSR:
355 /* We only handle 1 byte */
356 ret = mt8173_nor_wr_sr(mt8173_nor, *buf);
357 break;
358 default:
359 ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, buf, len, NULL, 0);
360 if (ret)
361 dev_warn(mt8173_nor->dev, "write reg failure!\n");
362 break;
363 }
364 return ret;
365 }
366
367 static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
368 struct device_node *flash_node)
369 {
370 struct mtd_part_parser_data ppdata = {
371 .of_node = flash_node,
372 };
373 int ret;
374 struct spi_nor *nor;
375 /* initialize controller to accept commands */
376 writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG);
377
378 nor = &mt8173_nor->nor;
379 nor->dev = mt8173_nor->dev;
380 nor->priv = mt8173_nor;
> 381 nor->flash_node = flash_node;
382
383 /* fill the hooks to spi nor */
384 nor->read = mt8173_nor_read;
385 nor->read_reg = mt8173_nor_read_reg;
386 nor->write = mt8173_nor_write;
387 nor->write_reg = mt8173_nor_write_reg;
> 388 nor->mtd.owner = THIS_MODULE;
> 389 nor->mtd.name = "mtk_nor";
390 /* initialized with NULL */
391 ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL);
392 if (ret)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Bayi, A few small comments. On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote: > add spi nor flash driver for mediatek controller > > Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com> > --- > drivers/mtd/spi-nor/Kconfig | 7 + > drivers/mtd/spi-nor/Makefile | 1 + > drivers/mtd/spi-nor/mtk-quadspi.c | 475 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 483 insertions(+) > create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c > diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile > index e53333e..0bf3a7f8 100644 > --- a/drivers/mtd/spi-nor/Makefile > +++ b/drivers/mtd/spi-nor/Makefile > @@ -1,3 +1,4 @@ > obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o > obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o > +obj-$(CONFIG_MTD_MT81xx_NOR) += mtk-quadspi.o > obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o > diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c > new file mode 100644 > index 0000000..6582811 > --- /dev/null > +++ b/drivers/mtd/spi-nor/mtk-quadspi.c > @@ -0,0 +1,475 @@ ... > +/* Can shift up to 48 bits (6 bytes) of TX/RX */ > +#define MTK_NOR_MAX_SHIFT 6 ... > +static int mt8173_nor_do_tx_rx(struct mt8173_nor *mt8173_nor, u8 op, > + u8 *tx, int txlen, u8 *rx, int rxlen) > +{ > + int len = 1 + txlen + rxlen; > + int i, ret, idx; > + > + if (len > MTK_NOR_MAX_SHIFT + 1) > + return -EINVAL; So I see you adjusted these bounds to add 1, which inspired one of my questions on the cover letter. Why do you allow len=7, which means you'd program 7*8 to the COUNT register, when the SoC manual says it has a max of 48? Is the manual wrong? I notice you added the '+ 1' to your driver, so it allows: do_tx_rx( txlen = 0 , rxlen = 6) /* e.g., for READ ID */ but that means your driver also allows: do_tx_rx( txlen = 6, rxlen = 0) /* ERROR: this will allow out of bounds write on PRGDATA register -1 */ If I understand correctly, the following constraints are more correct: /* Can only transmit 1 opcode and 5 other bytes */ if (1 + txlen > MTK_NOR_MAX_SHIFT) return -EINVAL; /* Can only receive 6 bytes after the opcode */ if (rxlen > MTK_NOR_MAX_SHIFT) return -EINVAL; /* Can only handle XXX bytes total */ /* How many bytes is the max for register MTK_NOR_CNT_REG ? */ if (len > XXX) return -EINVAL; > + > + writeb(len * 8, mt8173_nor->base + MTK_NOR_CNT_REG); > + > + /* start at PRGDATA5, go down to PRGDATA0 */ > + idx = MTK_NOR_MAX_SHIFT - 1; > + > + /* opcode */ > + writeb(op, mt8173_nor->base + MTK_NOR_PRG_REG(idx)); > + idx--; > + > + /* program TX data */ > + for (i = 0; i < txlen; i++, idx--) > + writeb(tx[i], mt8173_nor->base + MTK_NOR_PRG_REG(idx)); > + > + /* clear out rest of TX registers */ > + while (idx >= 0) { > + writeb(0, mt8173_nor->base + MTK_NOR_PRG_REG(idx)); > + idx--; > + } > + > + ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD); > + if (ret) > + return ret; > + > + /* restart at first RX byte */ > + idx = rxlen - 1; > + > + /* read out RX data */ > + for (i = 0; i < rxlen; i++, idx--) > + rx[i] = readb(mt8173_nor->base + MTK_NOR_SHREG(idx)); > + > + return 0; > +} > + ... > +static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor, > + struct device_node *flash_node) > +{ > + struct mtd_part_parser_data ppdata = { > + .of_node = flash_node, > + }; > + int ret; > + struct spi_nor *nor; Normally we'd have a blank line here. > + /* initialize controller to accept commands */ > + writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG); > + > + nor = &mt8173_nor->nor; > + nor->dev = mt8173_nor->dev; > + nor->priv = mt8173_nor; > + nor->flash_node = flash_node; > + > + /* fill the hooks to spi nor */ > + nor->read = mt8173_nor_read; > + nor->read_reg = mt8173_nor_read_reg; > + nor->write = mt8173_nor_write; > + nor->write_reg = mt8173_nor_write_reg; > + nor->mtd.owner = THIS_MODULE; > + nor->mtd.name = "mtk_nor"; > + /* initialized with NULL */ > + ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL); > + if (ret) > + return ret; > + > + return mtd_device_parse_register(&nor->mtd, NULL, &ppdata, NULL, 0); > +} ... Brian
On Mon, 2015-11-09 at 19:01 -0800, Brian Norris wrote: > Hi Bayi, > > A few small comments. > > On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote: > > add spi nor flash driver for mediatek controller > > > > Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com> > > --- > > drivers/mtd/spi-nor/Kconfig | 7 + > > drivers/mtd/spi-nor/Makefile | 1 + > > drivers/mtd/spi-nor/mtk-quadspi.c | 475 ++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 483 insertions(+) > > create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c > > > > diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile > > index e53333e..0bf3a7f8 100644 > > --- a/drivers/mtd/spi-nor/Makefile > > +++ b/drivers/mtd/spi-nor/Makefile > > @@ -1,3 +1,4 @@ > > obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o > > obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o > > +obj-$(CONFIG_MTD_MT81xx_NOR) += mtk-quadspi.o > > obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o > > diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c > > new file mode 100644 > > index 0000000..6582811 > > --- /dev/null > > +++ b/drivers/mtd/spi-nor/mtk-quadspi.c > > @@ -0,0 +1,475 @@ > > ... > > > +/* Can shift up to 48 bits (6 bytes) of TX/RX */ > > +#define MTK_NOR_MAX_SHIFT 6 > > ... > > > +static int mt8173_nor_do_tx_rx(struct mt8173_nor *mt8173_nor, u8 op, > > + u8 *tx, int txlen, u8 *rx, int rxlen) > > +{ > > + int len = 1 + txlen + rxlen; > > + int i, ret, idx; > > + > > + if (len > MTK_NOR_MAX_SHIFT + 1) > > + return -EINVAL; > > So I see you adjusted these bounds to add 1, which inspired one of my > questions on the cover letter. > > Why do you allow len=7, which means you'd program 7*8 to the COUNT > register, when the SoC manual says it has a max of 48? Is the manual > wrong? > Hi Brian, you are right, the manual is wrong here, Actually, it has a max of 56. when we want to read 6 IDs, we need transfer 1 byte command and 6 bytes null address to nor flash, then we can read the six IDs from SHREGx register. > I notice you added the '+ 1' to your driver, so it allows: > > do_tx_rx( txlen = 0 , rxlen = 6) /* e.g., for READ ID */ > > but that means your driver also allows: > > do_tx_rx( txlen = 6, rxlen = 0) /* ERROR: this will allow out of > bounds write on PRGDATA > register -1 */ > > If I understand correctly, the following constraints are more correct: > > /* Can only transmit 1 opcode and 5 other bytes */ > if (1 + txlen > MTK_NOR_MAX_SHIFT) > return -EINVAL; > > /* Can only receive 6 bytes after the opcode */ > if (rxlen > MTK_NOR_MAX_SHIFT) > return -EINVAL; > > /* Can only handle XXX bytes total */ > /* How many bytes is the max for register MTK_NOR_CNT_REG ? */ > if (len > XXX) > return -EINVAL; > yes, your constraints seems more correct, and I will adapt these lines to next patch. > > + > > + writeb(len * 8, mt8173_nor->base + MTK_NOR_CNT_REG); > > + > > + /* start at PRGDATA5, go down to PRGDATA0 */ > > + idx = MTK_NOR_MAX_SHIFT - 1; > > + > > + /* opcode */ > > + writeb(op, mt8173_nor->base + MTK_NOR_PRG_REG(idx)); > > + idx--; > > + > > + /* program TX data */ > > + for (i = 0; i < txlen; i++, idx--) > > + writeb(tx[i], mt8173_nor->base + MTK_NOR_PRG_REG(idx)); > > + > > + /* clear out rest of TX registers */ > > + while (idx >= 0) { > > + writeb(0, mt8173_nor->base + MTK_NOR_PRG_REG(idx)); > > + idx--; > > + } > > + > > + ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD); > > + if (ret) > > + return ret; > > + > > + /* restart at first RX byte */ > > + idx = rxlen - 1; > > + > > + /* read out RX data */ > > + for (i = 0; i < rxlen; i++, idx--) > > + rx[i] = readb(mt8173_nor->base + MTK_NOR_SHREG(idx)); > > + > > + return 0; > > +} > > + > > ... > > > +static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor, > > + struct device_node *flash_node) > > +{ > > + struct mtd_part_parser_data ppdata = { > > + .of_node = flash_node, > > + }; > > + int ret; > > + struct spi_nor *nor; > > Normally we'd have a blank line here. > Ok, I will fix it, and thanks for your advice. > > + /* initialize controller to accept commands */ > > + writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG); > > + > > + nor = &mt8173_nor->nor; > > + nor->dev = mt8173_nor->dev; > > + nor->priv = mt8173_nor; > > + nor->flash_node = flash_node; > > + > > + /* fill the hooks to spi nor */ > > + nor->read = mt8173_nor_read; > > + nor->read_reg = mt8173_nor_read_reg; > > + nor->write = mt8173_nor_write; > > + nor->write_reg = mt8173_nor_write_reg; > > + nor->mtd.owner = THIS_MODULE; > > + nor->mtd.name = "mtk_nor"; > > + /* initialized with NULL */ > > + ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL); > > + if (ret) > > + return ret; > > + > > + return mtd_device_parse_register(&nor->mtd, NULL, &ppdata, NULL, 0); > > +} > > ... > > Brian > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
One more small comment, since you're respinning this: On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote: > diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c > new file mode 100644 > index 0000000..6582811 > --- /dev/null > +++ b/drivers/mtd/spi-nor/mtk-quadspi.c > @@ -0,0 +1,475 @@ ... > +static int mtk_nor_drv_probe(struct platform_device *pdev) > +{ > + struct device_node *flash_np; > + struct resource *res; > + int ret; > + struct mt8173_nor *mt8173_nor; > + > + if (!pdev->dev.of_node) { > + dev_err(&pdev->dev, "No DT found\n"); > + return -EINVAL; > + } > + > + mt8173_nor = devm_kzalloc(&pdev->dev, sizeof(*mt8173_nor), GFP_KERNEL); > + if (!mt8173_nor) > + return -ENOMEM; > + platform_set_drvdata(pdev, mt8173_nor); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + mt8173_nor->base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(mt8173_nor->base)) > + return PTR_ERR(mt8173_nor->base); > + > + mt8173_nor->spi_clk = devm_clk_get(&pdev->dev, "spi"); > + if (IS_ERR(mt8173_nor->spi_clk)) { > + ret = PTR_ERR(mt8173_nor->spi_clk); > + goto nor_free; > + } > + > + mt8173_nor->nor_clk = devm_clk_get(&pdev->dev, "sf"); > + if (IS_ERR(mt8173_nor->nor_clk)) { > + ret = PTR_ERR(mt8173_nor->nor_clk); > + goto nor_free; > + } > + > + mt8173_nor->dev = &pdev->dev; > + clk_prepare_enable(mt8173_nor->spi_clk); > + clk_prepare_enable(mt8173_nor->nor_clk); You enable the clocks here, but... > + > + /* only support one attached flash */ > + flash_np = of_get_next_available_child(pdev->dev.of_node, NULL); > + if (!flash_np) { ... you might bail out here if there is no available flash node, and you never disable the clocks. (Same is true if we fail to detect the flash; you leave the no-longer-needed clocks enabled.) Seems like maybe you should disable clocks on failure. > + dev_err(&pdev->dev, "no SPI flash device to configure\n"); > + ret = -ENODEV; > + goto nor_free; > + } > + ret = mtk_nor_init(mt8173_nor, flash_np); > + > +nor_free: > + return ret; > +} Brian
On Wed, 2015-11-11 at 13:41 -0800, Brian Norris wrote: > One more small comment, since you're respinning this: > > On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote: > > diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c > > new file mode 100644 > > index 0000000..6582811 > > --- /dev/null > > +++ b/drivers/mtd/spi-nor/mtk-quadspi.c > > @@ -0,0 +1,475 @@ > > ... > > > +static int mtk_nor_drv_probe(struct platform_device *pdev) > > +{ > > + struct device_node *flash_np; > > + struct resource *res; > > + int ret; > > + struct mt8173_nor *mt8173_nor; > > + > > + if (!pdev->dev.of_node) { > > + dev_err(&pdev->dev, "No DT found\n"); > > + return -EINVAL; > > + } > > + > > + mt8173_nor = devm_kzalloc(&pdev->dev, sizeof(*mt8173_nor), GFP_KERNEL); > > + if (!mt8173_nor) > > + return -ENOMEM; > > + platform_set_drvdata(pdev, mt8173_nor); > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + mt8173_nor->base = devm_ioremap_resource(&pdev->dev, res); > > + if (IS_ERR(mt8173_nor->base)) > > + return PTR_ERR(mt8173_nor->base); > > + > > + mt8173_nor->spi_clk = devm_clk_get(&pdev->dev, "spi"); > > + if (IS_ERR(mt8173_nor->spi_clk)) { > > + ret = PTR_ERR(mt8173_nor->spi_clk); > > + goto nor_free; > > + } > > + > > + mt8173_nor->nor_clk = devm_clk_get(&pdev->dev, "sf"); > > + if (IS_ERR(mt8173_nor->nor_clk)) { > > + ret = PTR_ERR(mt8173_nor->nor_clk); > > + goto nor_free; > > + } > > + > > + mt8173_nor->dev = &pdev->dev; > > + clk_prepare_enable(mt8173_nor->spi_clk); > > + clk_prepare_enable(mt8173_nor->nor_clk); > > You enable the clocks here, but... > > > + > > + /* only support one attached flash */ > > + flash_np = of_get_next_available_child(pdev->dev.of_node, NULL); > > + if (!flash_np) { > > ... you might bail out here if there is no available flash node, and you > never disable the clocks. (Same is true if we fail to detect the flash; > you leave the no-longer-needed clocks enabled.) Seems like maybe you > should disable clocks on failure. Yes, I have forgot to disable these clocks on failure. and I will fix it in the next patch! Thanks! > > > + dev_err(&pdev->dev, "no SPI flash device to configure\n"); > > + ret = -ENODEV; > > + goto nor_free; > > + } > > + ret = mtk_nor_init(mt8173_nor, flash_np); > > + > > +nor_free: > > + return ret; > > +} > > Brian
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig index 89bf4c1..f544bbe 100644 --- a/drivers/mtd/spi-nor/Kconfig +++ b/drivers/mtd/spi-nor/Kconfig @@ -7,6 +7,13 @@ menuconfig MTD_SPI_NOR if MTD_SPI_NOR +config MTD_MT81xx_NOR + tristate "Mediatek MT81xx SPI NOR flash controller" + help + This enables access to SPI NOR flash, using MT81xx SPI NOR flash + controller. This controller does not support generic SPI BUS, it only + supports SPI NOR Flash. + config MTD_SPI_NOR_USE_4K_SECTORS bool "Use small 4096 B erase sectors" default y diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile index e53333e..0bf3a7f8 100644 --- a/drivers/mtd/spi-nor/Makefile +++ b/drivers/mtd/spi-nor/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o +obj-$(CONFIG_MTD_MT81xx_NOR) += mtk-quadspi.o obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c new file mode 100644 index 0000000..6582811 --- /dev/null +++ b/drivers/mtd/spi-nor/mtk-quadspi.c @@ -0,0 +1,475 @@ +/* + * Copyright (c) 2015 MediaTek Inc. + * Author: Bayi Cheng <bayi.cheng@mediatek.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/init.h> +#include <linux/io.h> +#include <linux/iopoll.h> +#include <linux/ioport.h> +#include <linux/math64.h> +#include <linux/module.h> +#include <linux/mtd/mtd.h> +#include <linux/mutex.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/pinctrl/consumer.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/partitions.h> +#include <linux/mtd/spi-nor.h> + +#define MTK_NOR_CMD_REG 0x00 +#define MTK_NOR_CNT_REG 0x04 +#define MTK_NOR_RDSR_REG 0x08 +#define MTK_NOR_RDATA_REG 0x0c +#define MTK_NOR_RADR0_REG 0x10 +#define MTK_NOR_RADR1_REG 0x14 +#define MTK_NOR_RADR2_REG 0x18 +#define MTK_NOR_WDATA_REG 0x1c +#define MTK_NOR_PRGDATA0_REG 0x20 +#define MTK_NOR_PRGDATA1_REG 0x24 +#define MTK_NOR_PRGDATA2_REG 0x28 +#define MTK_NOR_PRGDATA3_REG 0x2c +#define MTK_NOR_PRGDATA4_REG 0x30 +#define MTK_NOR_PRGDATA5_REG 0x34 +#define MTK_NOR_SHREG0_REG 0x38 +#define MTK_NOR_SHREG1_REG 0x3c +#define MTK_NOR_SHREG2_REG 0x40 +#define MTK_NOR_SHREG3_REG 0x44 +#define MTK_NOR_SHREG4_REG 0x48 +#define MTK_NOR_SHREG5_REG 0x4c +#define MTK_NOR_SHREG6_REG 0x50 +#define MTK_NOR_SHREG7_REG 0x54 +#define MTK_NOR_SHREG8_REG 0x58 +#define MTK_NOR_SHREG9_REG 0x5c +#define MTK_NOR_CFG1_REG 0x60 +#define MTK_NOR_CFG2_REG 0x64 +#define MTK_NOR_CFG3_REG 0x68 +#define MTK_NOR_STATUS0_REG 0x70 +#define MTK_NOR_STATUS1_REG 0x74 +#define MTK_NOR_STATUS2_REG 0x78 +#define MTK_NOR_STATUS3_REG 0x7c +#define MTK_NOR_FLHCFG_REG 0x84 +#define MTK_NOR_TIME_REG 0x94 +#define MTK_NOR_PP_DATA_REG 0x98 +#define MTK_NOR_PREBUF_STUS_REG 0x9c +#define MTK_NOR_DELSEL0_REG 0xa0 +#define MTK_NOR_DELSEL1_REG 0xa4 +#define MTK_NOR_INTRSTUS_REG 0xa8 +#define MTK_NOR_INTREN_REG 0xac +#define MTK_NOR_CHKSUM_CTL_REG 0xb8 +#define MTK_NOR_CHKSUM_REG 0xbc +#define MTK_NOR_CMD2_REG 0xc0 +#define MTK_NOR_WRPROT_REG 0xc4 +#define MTK_NOR_RADR3_REG 0xc8 +#define MTK_NOR_DUAL_REG 0xcc +#define MTK_NOR_DELSEL2_REG 0xd0 +#define MTK_NOR_DELSEL3_REG 0xd4 +#define MTK_NOR_DELSEL4_REG 0xd8 + +/* commands for mtk nor controller */ +#define MTK_NOR_READ_CMD 0x0 +#define MTK_NOR_RDSR_CMD 0x2 +#define MTK_NOR_PRG_CMD 0x4 +#define MTK_NOR_WR_CMD 0x10 +#define MTK_NOR_PIO_WR_CMD 0x90 +#define MTK_NOR_WRSR_CMD 0x20 +#define MTK_NOR_PIO_READ_CMD 0x81 +#define MTK_NOR_WR_BUF_ENABLE 0x1 +#define MTK_NOR_WR_BUF_DISABLE 0x0 +#define MTK_NOR_ENABLE_SF_CMD 0x30 +#define MTK_NOR_DUAD_ADDR_EN 0x8 +#define MTK_NOR_QUAD_READ_EN 0x4 +#define MTK_NOR_DUAL_ADDR_EN 0x2 +#define MTK_NOR_DUAL_READ_EN 0x1 +#define MTK_NOR_DUAL_DISABLE 0x0 +#define MTK_NOR_FAST_READ 0x1 + +#define SFLASH_WRBUF_SIZE 128 + +/* Can shift up to 48 bits (6 bytes) of TX/RX */ +#define MTK_NOR_MAX_SHIFT 6 +/* Helpers for accessing the program data / shift data registers */ +#define MTK_NOR_PRG_REG(n) (MTK_NOR_PRGDATA0_REG + 4 * (n)) +#define MTK_NOR_SHREG(n) (MTK_NOR_SHREG0_REG + 4 * (n)) + +struct mt8173_nor { + struct spi_nor nor; + struct device *dev; + void __iomem *base; /* nor flash base address */ + struct clk *spi_clk; + struct clk *nor_clk; +}; + +static void mt8173_nor_set_read_mode(struct mt8173_nor *mt8173_nor) +{ + struct spi_nor *nor = &mt8173_nor->nor; + + writeb(nor->read_opcode, mt8173_nor->base + MTK_NOR_PRGDATA3_REG); + + switch (nor->flash_read) { + case SPI_NOR_FAST: + writeb(MTK_NOR_FAST_READ, mt8173_nor->base + + MTK_NOR_CFG1_REG); + break; + case SPI_NOR_DUAL: + writeb(MTK_NOR_DUAL_READ_EN, mt8173_nor->base + + MTK_NOR_DUAL_REG); + break; + case SPI_NOR_QUAD: + writeb(MTK_NOR_QUAD_READ_EN, mt8173_nor->base + + MTK_NOR_DUAL_REG); + break; + default: + writeb(MTK_NOR_DUAL_DISABLE, mt8173_nor->base + + MTK_NOR_DUAL_REG); + break; + } +} + +static int mt8173_nor_execute_cmd(struct mt8173_nor *mt8173_nor, u8 cmdval) +{ + int reg; + u8 val = cmdval & 0x1f; + + writeb(cmdval, mt8173_nor->base + MTK_NOR_CMD_REG); + return readl_poll_timeout(mt8173_nor->base + MTK_NOR_CMD_REG, reg, + !(reg & val), 100, 10000); +} + +static int mt8173_nor_do_tx_rx(struct mt8173_nor *mt8173_nor, u8 op, + u8 *tx, int txlen, u8 *rx, int rxlen) +{ + int len = 1 + txlen + rxlen; + int i, ret, idx; + + if (len > MTK_NOR_MAX_SHIFT + 1) + return -EINVAL; + + writeb(len * 8, mt8173_nor->base + MTK_NOR_CNT_REG); + + /* start at PRGDATA5, go down to PRGDATA0 */ + idx = MTK_NOR_MAX_SHIFT - 1; + + /* opcode */ + writeb(op, mt8173_nor->base + MTK_NOR_PRG_REG(idx)); + idx--; + + /* program TX data */ + for (i = 0; i < txlen; i++, idx--) + writeb(tx[i], mt8173_nor->base + MTK_NOR_PRG_REG(idx)); + + /* clear out rest of TX registers */ + while (idx >= 0) { + writeb(0, mt8173_nor->base + MTK_NOR_PRG_REG(idx)); + idx--; + } + + ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD); + if (ret) + return ret; + + /* restart at first RX byte */ + idx = rxlen - 1; + + /* read out RX data */ + for (i = 0; i < rxlen; i++, idx--) + rx[i] = readb(mt8173_nor->base + MTK_NOR_SHREG(idx)); + + return 0; +} + +/* Do a WRSR (Write Status Register) command */ +static int mt8173_nor_wr_sr(struct mt8173_nor *mt8173_nor, u8 sr) +{ + writeb(sr, mt8173_nor->base + MTK_NOR_PRGDATA5_REG); + writeb(8, mt8173_nor->base + MTK_NOR_CNT_REG); + return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_WRSR_CMD); +} + +static int mt8173_nor_write_buffer_enable(struct mt8173_nor *mt8173_nor) +{ + u8 reg; + + /* the bit0 of MTK_NOR_CFG2_REG is pre-fetch buffer + * 0: pre-fetch buffer use for read + * 1: pre-fetch buffer use for page program + */ + writel(MTK_NOR_WR_BUF_ENABLE, mt8173_nor->base + MTK_NOR_CFG2_REG); + return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg, + 0x01 == (reg & 0x01), 100, 10000); +} + +static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor) +{ + u8 reg; + + writel(MTK_NOR_WR_BUF_DISABLE, mt8173_nor->base + MTK_NOR_CFG2_REG); + return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg, + MTK_NOR_WR_BUF_DISABLE == (reg & 0x1), 100, + 10000); +} + +static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr) +{ + int i; + + for (i = 0; i < 3; i++) { + writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4); + addr >>= 8; + } + /* Last register is non-contiguous */ + writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR3_REG); +} + +static int mt8173_nor_read(struct spi_nor *nor, loff_t from, size_t length, + size_t *retlen, u_char *buffer) +{ + int i, ret; + int addr = (int)from; + u8 *buf = (u8 *)buffer; + struct mt8173_nor *mt8173_nor = nor->priv; + + /* set mode for fast read mode ,dual mode or quad mode */ + mt8173_nor_set_read_mode(mt8173_nor); + mt8173_nor_set_addr(mt8173_nor, addr); + + for (i = 0; i < length; i++, (*retlen)++) { + ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_READ_CMD); + if (ret < 0) + return ret; + buf[i] = readb(mt8173_nor->base + MTK_NOR_RDATA_REG); + } + return 0; +} + +static int mt8173_nor_write_single_byte(struct mt8173_nor *mt8173_nor, + int addr, int length, u8 *data) +{ + int i, ret; + + mt8173_nor_set_addr(mt8173_nor, addr); + + for (i = 0; i < length; i++) { + ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_WR_CMD); + if (ret < 0) + return ret; + writeb(*data++, mt8173_nor->base + MTK_NOR_WDATA_REG); + } + return 0; +} + +static int mt8173_nor_write_buffer(struct mt8173_nor *mt8173_nor, int addr, + const u8 *buf) +{ + int i, bufidx, data; + + mt8173_nor_set_addr(mt8173_nor, addr); + + bufidx = 0; + for (i = 0; i < SFLASH_WRBUF_SIZE; i += 4) { + data = buf[bufidx + 3]<<24 | buf[bufidx + 2]<<16 | + buf[bufidx + 1]<<8 | buf[bufidx]; + bufidx += 4; + writel(data, mt8173_nor->base + MTK_NOR_PP_DATA_REG); + } + return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_WR_CMD); +} + +static void mt8173_nor_write(struct spi_nor *nor, loff_t to, size_t len, + size_t *retlen, const u_char *buf) +{ + int ret; + struct mt8173_nor *mt8173_nor = nor->priv; + + ret = mt8173_nor_write_buffer_enable(mt8173_nor); + if (ret < 0) + dev_warn(mt8173_nor->dev, "write buffer enable failed!\n"); + + while (len >= SFLASH_WRBUF_SIZE) { + ret = mt8173_nor_write_buffer(mt8173_nor, to, buf); + if (ret < 0) + dev_err(mt8173_nor->dev, "write buffer failed!\n"); + len -= SFLASH_WRBUF_SIZE; + to += SFLASH_WRBUF_SIZE; + buf += SFLASH_WRBUF_SIZE; + (*retlen) += SFLASH_WRBUF_SIZE; + } + ret = mt8173_nor_write_buffer_disable(mt8173_nor); + if (ret < 0) + dev_warn(mt8173_nor->dev, "write buffer disable failed!\n"); + + if (len) { + ret = mt8173_nor_write_single_byte(mt8173_nor, to, (int)len, + (u8 *)buf); + if (ret < 0) + dev_err(mt8173_nor->dev, "write single byte failed!\n"); + (*retlen) += len; + } +} + +static int mt8173_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len) +{ + int ret; + struct mt8173_nor *mt8173_nor = nor->priv; + + switch (opcode) { + case SPINOR_OP_RDSR: + ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_RDSR_CMD); + if (ret < 0) + return ret; + if (len == 1) + *buf = readb(mt8173_nor->base + MTK_NOR_RDSR_REG); + else + dev_err(mt8173_nor->dev, "len should be 1 for read status!\n"); + break; + default: + ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, NULL, 0, buf, len); + break; + } + return ret; +} + +static int mt8173_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, + int len) +{ + int ret; + struct mt8173_nor *mt8173_nor = nor->priv; + + switch (opcode) { + case SPINOR_OP_WRSR: + /* We only handle 1 byte */ + ret = mt8173_nor_wr_sr(mt8173_nor, *buf); + break; + default: + ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, buf, len, NULL, 0); + if (ret) + dev_warn(mt8173_nor->dev, "write reg failure!\n"); + break; + } + return ret; +} + +static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor, + struct device_node *flash_node) +{ + struct mtd_part_parser_data ppdata = { + .of_node = flash_node, + }; + int ret; + struct spi_nor *nor; + /* initialize controller to accept commands */ + writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG); + + nor = &mt8173_nor->nor; + nor->dev = mt8173_nor->dev; + nor->priv = mt8173_nor; + nor->flash_node = flash_node; + + /* fill the hooks to spi nor */ + nor->read = mt8173_nor_read; + nor->read_reg = mt8173_nor_read_reg; + nor->write = mt8173_nor_write; + nor->write_reg = mt8173_nor_write_reg; + nor->mtd.owner = THIS_MODULE; + nor->mtd.name = "mtk_nor"; + /* initialized with NULL */ + ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL); + if (ret) + return ret; + + return mtd_device_parse_register(&nor->mtd, NULL, &ppdata, NULL, 0); +} + +static int mtk_nor_drv_probe(struct platform_device *pdev) +{ + struct device_node *flash_np; + struct resource *res; + int ret; + struct mt8173_nor *mt8173_nor; + + if (!pdev->dev.of_node) { + dev_err(&pdev->dev, "No DT found\n"); + return -EINVAL; + } + + mt8173_nor = devm_kzalloc(&pdev->dev, sizeof(*mt8173_nor), GFP_KERNEL); + if (!mt8173_nor) + return -ENOMEM; + platform_set_drvdata(pdev, mt8173_nor); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + mt8173_nor->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(mt8173_nor->base)) + return PTR_ERR(mt8173_nor->base); + + mt8173_nor->spi_clk = devm_clk_get(&pdev->dev, "spi"); + if (IS_ERR(mt8173_nor->spi_clk)) { + ret = PTR_ERR(mt8173_nor->spi_clk); + goto nor_free; + } + + mt8173_nor->nor_clk = devm_clk_get(&pdev->dev, "sf"); + if (IS_ERR(mt8173_nor->nor_clk)) { + ret = PTR_ERR(mt8173_nor->nor_clk); + goto nor_free; + } + + mt8173_nor->dev = &pdev->dev; + clk_prepare_enable(mt8173_nor->spi_clk); + clk_prepare_enable(mt8173_nor->nor_clk); + + /* only support one attached flash */ + flash_np = of_get_next_available_child(pdev->dev.of_node, NULL); + if (!flash_np) { + dev_err(&pdev->dev, "no SPI flash device to configure\n"); + ret = -ENODEV; + goto nor_free; + } + ret = mtk_nor_init(mt8173_nor, flash_np); + +nor_free: + return ret; +} + +static int mtk_nor_drv_remove(struct platform_device *pdev) +{ + struct mt8173_nor *mt8173_nor = platform_get_drvdata(pdev); + + clk_disable_unprepare(mt8173_nor->spi_clk); + clk_disable_unprepare(mt8173_nor->nor_clk); + return 0; +} + +static const struct of_device_id mtk_nor_of_ids[] = { + { .compatible = "mediatek,mt8173-nor"}, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, mtk_nor_of_ids); + +static struct platform_driver mtk_nor_driver = { + .probe = mtk_nor_drv_probe, + .remove = mtk_nor_drv_remove, + .driver = { + .name = "mtk-nor", + .of_match_table = mtk_nor_of_ids, + }, +}; + +module_platform_driver(mtk_nor_driver); +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("MediaTek SPI NOR Flash Driver");
add spi nor flash driver for mediatek controller Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com> --- drivers/mtd/spi-nor/Kconfig | 7 + drivers/mtd/spi-nor/Makefile | 1 + drivers/mtd/spi-nor/mtk-quadspi.c | 475 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 483 insertions(+) create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c