Re: [v2] mtd: spi-nor: parse SFDP 4-byte Address Instruction Table

2018-12-08 Thread Boris Brezillon
On Thu, 2018-12-06 at 14:43:39 UTC,  wrote:
> From: Cyrille Pitchen 
> 
> Add support for SFDP (JESD216B) 4-byte Address Instruction Table. This
> table is optional but when available, we parse it to get the 4-byte
> address op codes supported by the memory.
> Using these op codes is stateless as opposed to entering the 4-byte
> address mode or setting the Base Address Register (BAR).
> 
> Flashes that have the 4BAIT table declared can now support
> SPINOR_OP_PP_1_1_4_4B and SPINOR_OP_PP_1_4_4_4B opcodes.
> 
> Tested on MX25L25673G.
> 
> Signed-off-by: Cyrille Pitchen 
> [tudor.amba...@microchip.com:
> - rework erase and page program logic,
> - pass DMA-able buffer to spi_nor_read_sfdp(),
> - introduce SPI_NOR_HAS_4BAIT
> - various minor updates.]
> Signed-off-by: Tudor Ambarus 

Applied to http://git.infradead.org/linux-mtd.git spi-nor/next, thanks.

Boris


Re: mtd: spi-nor: Add 4B_OPCODES flag to is25lp256

2018-12-08 Thread Boris Brezillon
On Wed, 2018-11-14 at 12:55:18 UTC, Liu Xiang wrote:
> The is25lp256 supports 4-byte opcodes and quad output.
> 
> Suggested-by: Boris Brezillon 
> Signed-off-by: Liu Xiang 
> Reviewed-by: Tudor Ambarus 

Applied to http://git.infradead.org/linux-mtd.git spi-nor/next, thanks.

Boris


Re: [PATCH v2] mtd: spi-nor: parse SFDP 4-byte Address Instruction Table

2018-12-07 Thread Boris Brezillon
Hi Tudor,

On Thu, 6 Dec 2018 14:43:39 +
 wrote:

>  /**
>   * spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
>   * @nor: pointer to a 'struct spi_nor'
> @@ -3276,6 +3462,10 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
>   err = spi_nor_parse_smpt(nor, param_header);
>   break;
>  
> + case SFDP_4BAIT_ID:
> + err = spi_nor_parse_4bait(nor, param_header, params);
> + break;
> +
>   default:
>   break;
>   }
> @@ -3857,7 +4047,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>   (JEDEC_MFR(info) == SNOR_MFR_SPANSION && mtd->size > SZ_16M))
>   nor->flags |= SNOR_F_4B_OPCODES;
>  
> - if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES)
> + if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES &&
> + !(nor->flags & SNOR_F_HAS_4BAIT))
>   spi_nor_set_4byte_opcodes(nor);

The list of checks is growing. Maybe we could move those checks
at the beginning of spi_nor_set_4byte_opcodes() so we can
unconditionally call spi_nor_set_4byte_opcodes() here.
Not something I ask you to fix in this patch, just thinking out loud.

The rest of the patch looks good, but I'm pretty sure we'll run into
trouble as soon as we start parsing this 4BAIT section (as has been the
case for all other SFPD sections so far), just because vendors tend to
get this sort of things wrong. I don't have a solution for that other
than the proposed SFDP fixup hooks infrastructure [1] or the more
conservative "don't parse/trust SFDP" approach. Note that, even if we
go for the SFDP fixups approach, we might still break setups where the
4BAIT section of the SFDP table is broken until someone comes with a
fixup for those broken chips.

Since I don't like to block inclusion of new features for purely
theoretical issues, I'm in favor of applying this patch, but be
prepared to receive bug reports during the 4.21-rc cycle ;-).

Regards,

Boris

[1]http://patchwork.ozlabs.org/patch/1008687/


Re: [RESEND PATCH 2/4] mtd: spi-nor: mtk-quadspi: use ofpart for parsing partitions

2018-12-06 Thread Boris Brezillon
On Thu, 29 Nov 2018 14:29:54 +0800
Ryder Lee  wrote:

> From: Guochun Mao 
> 
> Replace mtd_device_register with mtd_device_parse_register for
> parsing partitions and add ofpart support.

What's the problem with the default partition parser table [1]?

[1]https://elixir.bootlin.com/linux/latest/source/drivers/mtd/mtdpart.c#L793


Re: [PATCH v5 0/7] spi: add support for octal mode

2018-12-05 Thread Boris Brezillon
On Thu, 6 Dec 2018 04:20:26 +
Yogesh Narayan Gaur  wrote:

> Hi Boris,
> 
> > -Original Message-
> > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> > Sent: Wednesday, December 5, 2018 6:16 PM
> > To: Vignesh R ; broo...@kernel.org
> > Cc: Yogesh Narayan Gaur ; linux-
> > m...@lists.infradead.org; marek.va...@gmail.com; linux-...@vger.kernel.org;
> > devicet...@vger.kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> > shawn...@kernel.org; linux-arm-ker...@lists.infradead.org;
> > computersforpe...@gmail.com; frieder.schre...@exceet.de; linux-
> > ker...@vger.kernel.org
> > Subject: Re: [PATCH v5 0/7] spi: add support for octal mode
> > 
> > On Wed, 5 Dec 2018 17:25:12 +0530
> > Vignesh R  wrote:
> >   
> > > >>   mtd: spi-nor: add opcodes for octal Read/Write commands
> > > >>   mtd: spi-nor: add octal read flag for flash mt35xu512aba  
> > >
> > > Could you consider merging these two patches alone for v4.21?
> > > These can be applied independent of other patches in the series and
> > > would allow supporting OSPI flash at SPI NOR level with Cadence QSPI 
> > > driver.  
> > 
> > Yep, I'll queue them to spi-nor/next.
> >   
> > > >>   spi: nxp-fspi: add octal mode flag bit for octal support  
> > 
> > Mark, I think you can pick this one too.  
> 
> This patch is dependent on the series [1] which is yet to be applied by you, 
> please apply.

By me? I can't apply SPI patches.


Re: [PATCH v5 0/7] spi: add support for octal mode

2018-12-05 Thread Boris Brezillon
On Wed, 5 Dec 2018 17:25:12 +0530
Vignesh R  wrote:

> >>   mtd: spi-nor: add opcodes for octal Read/Write commands
> >>   mtd: spi-nor: add octal read flag for flash mt35xu512aba  
> 
> Could you consider merging these two patches alone for v4.21?
> These can be applied independent of other patches in the series and
> would allow supporting OSPI flash at SPI NOR level with Cadence QSPI driver.

Yep, I'll queue them to spi-nor/next.

> >>   spi: nxp-fspi: add octal mode flag bit for octal support  

Mark, I think you can pick this one too.


Re: [PATCH] mips: annotate implicit fall throughs

2018-12-03 Thread Boris Brezillon
Hi Mathieu,

The subject prefix should be "mtd: rawnand: jz4780:" not "mips:"

Regards,

Boris

On Mon,  3 Dec 2018 22:22:13 +0100
Mathieu Malaterre  wrote:

> There is a plan to build the kernel with -Wimplicit-fallthrough and
> these places in the code produced warnings. Fix them up.
> 
> Signed-off-by: Mathieu Malaterre 
> ---
>  drivers/mtd/nand/raw/jz4780_bch.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/jz4780_bch.c 
> b/drivers/mtd/nand/raw/jz4780_bch.c
> index 731c6051d91e..7201827809e9 100644
> --- a/drivers/mtd/nand/raw/jz4780_bch.c
> +++ b/drivers/mtd/nand/raw/jz4780_bch.c
> @@ -136,8 +136,10 @@ static void jz4780_bch_read_parity(struct jz4780_bch 
> *bch, void *buf,
>   switch (size8) {
>   case 3:
>   dest8[2] = (val >> 16) & 0xff;
> + /* fall through */
>   case 2:
>   dest8[1] = (val >> 8) & 0xff;
> + /* fall through */
>   case 1:
>   dest8[0] = val & 0xff;
>   break;



Re: [PATCH v1 0/2] SPI-NOR add NPCM FIU controller driver

2018-12-03 Thread Boris Brezillon
Hi Tomer;

On Mon, 3 Dec 2018 16:09:19 +0200
Tomer Maimon  wrote:


> A few comments/input:
> 
> 
>1. We have been working on this driver for quite a long time to port it
>to the latest Linux conventions, polish the code, run tests and reach high
>quality.
>Our partners and customers are waiting to get this driver upstream so
>they can freely use it.

Your patch prefix says "v1", so I'm assuming this is the first public
version. I'm sure you spent a lot of time developing this driver
internally, but no matter how long it took, it's a first version for
us, and since we are moving away from the spi_nor controller interface,
I'm not willing to accept new drivers using this interface.

If you want more background about the spi_nor controller interface
deprecation, you can read [1].

>Since this driver is already in final stages and is in very good shape
>we will appreciate if you can review this specific driver/interface and
>help us to upstream it.

Actually, I'd like to do it the other way around: let you rework the
driver to implement the spi-mem interface and review this version.
Otherwise I'll be reviewing things I don't intend to merge anyway.

>2.  As for the new interface, we are open for any discussion and for
>porting the driver as required.
>We are unsure what is this specific interface and weather it really fits
>a driver for a Flash Interface Controller module (rather than a SPI flash
>device).

Didn't go through the code in details, but at first glance, it looks
like it would fit pretty well.

>Is it possible to get a sample driver from another Flash Interface
>Controller module that was ported to this new interface ?

We recently converted the atmel QSPI driver [2].

Regards,

Boris

[1]https://bootlin.com/blog/spi-mem-bringing-some-consistency-to-the-spi-memory-ecosystem/
[2]https://patchwork.kernel.org/project/spi-devel-general/list/?series=38347=*


Re: [v3] mtd: spi-nor: cast to u64 to avoid uint overflows

2018-12-03 Thread Boris Brezillon
On Mon,  3 Dec 2018 08:51:54 +0100
Boris Brezillon  wrote:

> On Wed, 2018-11-28 at 08:02:14 UTC, Huijin Park wrote:
> > From: "huijin.park" 
> > 
> > The "params->size" is defined as "u64".
> > And "info->sector_size" and "info->n_sectors" are defined as
> > unsigned int and u16.
> > Thus, u64 data might have strange data(loss data) if the result
> > overflows an unsigned int.
> > This patch casts "info->sector_size" to an u64.
> > 
> > Signed-off-by: huijin.park 
> > Reviewed-by: Geert Uytterhoeven   
> 
> Applied to http://git.infradead.org/linux-mtd.git mtd/next, thanks.

Oops, should I been queued to spi-nor/next. I dropped the patch and
will soon push it to spi-nor/next.


[PATCH] MAINTAINERS: Update my email address

2018-12-03 Thread Boris Brezillon
Use my korg address instead of the bootlin one.

Signed-off-by: Boris Brezillon 
---
 .mailmap| 7 ---
 MAINTAINERS | 8 
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/.mailmap b/.mailmap
index 28fecafa6506..b4b0b0b768dd 100644
--- a/.mailmap
+++ b/.mailmap
@@ -36,9 +36,10 @@ Bart Van Assche  

 Ben Gardner 
 Ben M Cahill 
 Björn Steinbrink 
-Boris Brezillon  

-Boris Brezillon  
-Boris Brezillon  
+Boris Brezillon  
+Boris Brezillon  
+Boris Brezillon  
+Boris Brezillon  
 Brian Avery 
 Brian King 
 Christoph Hellwig 
diff --git a/MAINTAINERS b/MAINTAINERS
index 0abecc528dac..a22b0c23f419 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4821,7 +4821,7 @@ F:Documentation/gpu/meson.rst
 T: git git://anongit.freedesktop.org/drm/drm-misc
 
 DRM DRIVERS FOR ATMEL HLCDC
-M: Boris Brezillon 
+M: Boris Brezillon 
 L: dri-de...@lists.freedesktop.org
 S: Supported
 F: drivers/gpu/drm/atmel-hlcdc/
@@ -8872,7 +8872,7 @@ F:include/uapi/drm/armada_drm.h
 F: Documentation/devicetree/bindings/display/armada/
 
 MARVELL CRYPTO DRIVER
-M: Boris Brezillon 
+M: Boris Brezillon 
 M: Arnaud Ebalard 
 F: drivers/crypto/marvell/
 S: Maintained
@@ -9576,7 +9576,7 @@ F:mm/
 MEMORY TECHNOLOGY DEVICES (MTD)
 M: David Woodhouse 
 M: Brian Norris 
-M: Boris Brezillon 
+M: Boris Brezillon 
 M: Marek Vasut 
 M: Richard Weinberger 
 L: linux-...@lists.infradead.org
@@ -10166,7 +10166,7 @@ S:  Supported
 F: drivers/net/ethernet/myricom/myri10ge/
 
 NAND FLASH SUBSYSTEM
-M: Boris Brezillon 
+M: Boris Brezillon 
 M: Miquel Raynal 
 R: Richard Weinberger 
 L: linux-...@lists.infradead.org
-- 
2.17.1



Re: [PATCH] i3c: Rename dw-i3c-master.c into i3c-master-dw.c

2018-12-03 Thread Boris Brezillon
Hi Vitor,

On Mon, 26 Nov 2018 09:45:14 +0100
Boris Brezillon  wrote:

> Follow the naming scheme introduced by the Cadence driver to keep things
> consistent.
> 
> Signed-off-by: Boris Brezillon 
> ---
>  drivers/i3c/master/Makefile | 2 +-
>  drivers/i3c/master/{dw-i3c-master.c => i3c-master-dw.c} | 0
>  2 files changed, 1 insertion(+), 1 deletion(-)
>  rename drivers/i3c/master/{dw-i3c-master.c => i3c-master-dw.c} (100%)

Any opinion on this rename. If you disagree with this, then I'd like to
know it.

Regards,

Boris

> 
> diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
> index fc53939a0bb1..c5f52733aa10 100644
> --- a/drivers/i3c/master/Makefile
> +++ b/drivers/i3c/master/Makefile
> @@ -1,2 +1,2 @@
>  obj-$(CONFIG_CDNS_I3C_MASTER)+= i3c-master-cdns.o
> -obj-$(CONFIG_DW_I3C_MASTER)  += dw-i3c-master.o
> +obj-$(CONFIG_DW_I3C_MASTER)  += i3c-master-dw.o
> diff --git a/drivers/i3c/master/dw-i3c-master.c 
> b/drivers/i3c/master/i3c-master-dw.c
> similarity index 100%
> rename from drivers/i3c/master/dw-i3c-master.c
> rename to drivers/i3c/master/i3c-master-dw.c



Re: [PATCH v1 0/2] SPI-NOR add NPCM FIU controller driver

2018-12-03 Thread Boris Brezillon
On Mon,  3 Dec 2018 11:14:54 +0200
Tomer Maimon  wrote:

> This patch set adds Flash Interface Unit(FIU) SPI-NOR
> support for the Nuvoton NPCM Baseboard Management 
> Controller (BMC).
> 
> The FIU supports single, dual or quad communication interface.
> 
> the FIU controller can operate in following modes:
> - User Mode Access(UMA): provides flash access by using an
>   indirect address/data mechanism.
> - direct rd/wr mode: maps the flash memory into the core
>   address space.
> - SPI-X mode: used for an expansion bus to an ASIC or CPLD.
> 
> The NPCM750/730/715/710 supports up to three FIU devices:
> - FIU0 supports two chip select.
> - FIU3 supports four chip select.
> - FIUX supports two chip select.
> 
> The NPCM FIU driver tested on NPCM750 evaluation board.
> 
> Tomer Maimon (2):
>   dt-binding: mtd: add NPCM FIU controller
>   mtd: spi-nor: add NPCM FIU controller driver
> 
>  Documentation/devicetree/bindings/mtd/npcm-fiu.txt |  64 ++
>  drivers/mtd/spi-nor/Kconfig|   8 +
>  drivers/mtd/spi-nor/Makefile   |   1 +
>  drivers/mtd/spi-nor/npcm-fiu.c | 930 
> +

We are currently trying to move all SPI NOR controller drivers out of
drivers/mtd/spi-nor. Can you try to implement the spi-mem interface [1]
and place your driver in drivers/spi/. 

[1]https://elixir.bootlin.com/linux/v4.20-rc5/source/include/linux/spi/spi-mem.h#L185


Re: [PATCH v5 0/7] spi: add support for octal mode

2018-12-03 Thread Boris Brezillon
Hi Mark,

On Mon, 3 Dec 2018 08:39:00 +
Yogesh Narayan Gaur  wrote:

> 
> Yogesh Gaur (7):
>   spi: add support for octal mode I/O data transfer
>   spi: spi-mem: add support for octal mode I/O data transfer

Can you take those 2 patches in your tree for 4.21/5.0?

>   mtd: spi-nor: add opcodes for octal Read/Write commands
>   mtd: spi-nor: add octal read flag for flash mt35xu512aba
>   mtd: m25p80: add support of octal mode I/O transfer
>   spi: nxp-fspi: add octal mode flag bit for octal support

I'll queue these ones after 4.21-rc1/5.0-rc1 is out (which means
they'll appear in 4.22/5.1).

>   arm64: dts: lx2160a: update fspi node


Re: [PATCH v4 1/7] spi: add support for octo mode I/O data transfer

2018-12-03 Thread Boris Brezillon
Hi Yogesh,

On Thu, 22 Nov 2018 05:14:31 +
Yogesh Narayan Gaur  wrote:

> Add flags for Octo mode I/O data transfer
> Required for the SPI controller which can do the data transfer (TX/RX)
> on 8 data lines e.g. NXP FlexSPI controller.
>  SPI_TX_OCTO: transmit with 8 wires
>  SPI_RX_OCTO: receive with 8 wires
> 
> Signed-off-by: Yogesh Gaur 
> Reviewed-by: Boris Brezillon 
> ---
> Changes for v4:
> - Rebase on top of v4.20-rc2
> Changes for v3:
> - Modified string 'octal' with 'octo'.

When I listed the differences between your version and mine, I
mentioned the OCTO vs OCTAL name, but I didn't say my decision was the
correct one :-). Looks like OCTAL is the term employed almost
everywhere, and it's also consistent with DUAL. Would you mind
sending a new version reverting the name to OCTAL.

Sorry for the unclear statement in my previous review.

Regards,

Boris


Re: jffs2: Fix use of uninitialized delayed_work, lockdep breakage

2018-12-02 Thread Boris Brezillon
On Fri, 2018-10-19 at 08:30:20 UTC, Daniel Santos wrote:
> jffs2_sync_fs makes the assumption that if CONFIG_JFFS2_FS_WRITEBUFFER
> is defined then a write buffer is available and has been initialized.
> However, this does is not the case when the mtd device has no
> out-of-band buffer:
> 
> int jffs2_nand_flash_setup(struct jffs2_sb_info *c)
> {
> if (!c->mtd->oobsize)
> return 0;
> ...
> 
> The resulting call to cancel_delayed_work_sync passing a uninitialized
> (but zeroed) delayed_work struct forces lockdep to become disabled.
> 
> [   90.050639] overlayfs: upper fs does not support tmpfile.
> [   90.652264] INFO: trying to register non-static key.
> [   90.662171] the code is fine but needs lockdep annotation.
> [   90.673090] turning off the locking correctness validator.
> [   90.684021] CPU: 0 PID: 1762 Comm: mount_root Not tainted 4.14.63 #0
> [   90.696672] Stack :   80d8f6a2 0038 805f 80444600 
> 8fe364f4 805dfbe7
> [   90.713349] 80563a30 06e2 8068370c 0001  0001 
> 8e2fdc48 
> [   90.730020]   80d9  0106  
> 6465746e 312e3420
> [   90.746690] 6b636f6c 03bf f800 20676e69  8000 
>  8e2c2a90
> [   90.763362] 80d9 0001  8e2c2a90 0003 80260dc0 
> 08052098 8068
> [   90.780033] ...
> [   90.784902] Call Trace:
> [   90.789793] [<8000f0d8>] show_stack+0xb8/0x148
> [   90.798659] [<8005a000>] register_lock_class+0x270/0x55c
> [   90.809247] [<8005cb64>] __lock_acquire+0x13c/0xf7c
> [   90.818964] [<8005e314>] lock_acquire+0x194/0x1dc
> [   90.828345] [<8003f27c>] flush_work+0x200/0x24c
> [   90.837374] [<80041dfc>] __cancel_work_timer+0x158/0x210
> [   90.847958] [<801a8770>] jffs2_sync_fs+0x20/0x54
> [   90.857173] [<80125cf4>] iterate_supers+0xf4/0x120
> [   90.866729] [<80158fc4>] sys_sync+0x44/0x9c
> [   90.875067] [<80014424>] syscall_common+0x34/0x58
> 
> Signed-off-by: Daniel Santos 
> Reviewed-by: Hou Tao 

Applied to http://git.infradead.org/linux-mtd.git mtd/next, thanks.

Boris


Re: mtd: nftl: clean up indentation, remove extraneous tabs

2018-12-02 Thread Boris Brezillon
On Sun, 2018-11-18 at 16:36:56 UTC, Colin King wrote:
> From: Colin Ian King 
> 
> The hunk of code is indented too much by one level, fix this by
> removing the extraneous tabs. Also terminate block comment using
> the recommended coding style to clean up checkpatch warning.
> 
> Signed-off-by: Colin Ian King 

Applied to http://git.infradead.org/linux-mtd.git mtd/next, thanks.

Boris


Re: [v3] mtd: change len type from signed to unsigned type

2018-12-02 Thread Boris Brezillon
On Thu, 2018-11-29 at 04:19:51 UTC, Huijin Park wrote:
> From: "huijin.park" 
> 
> Callers of erase_write() always pass an unsigned int.
> So this patch avoids a cast to an int.
> 
> Signed-off-by: huijin.park 
> Reviewed-by: Miquel Raynal 

Applied to http://git.infradead.org/linux-mtd.git mtd/next, thanks.

Boris


Re: [v3] mtd: spi-nor: cast to u64 to avoid uint overflows

2018-12-02 Thread Boris Brezillon
On Wed, 2018-11-28 at 08:02:14 UTC, Huijin Park wrote:
> From: "huijin.park" 
> 
> The "params->size" is defined as "u64".
> And "info->sector_size" and "info->n_sectors" are defined as
> unsigned int and u16.
> Thus, u64 data might have strange data(loss data) if the result
> overflows an unsigned int.
> This patch casts "info->sector_size" to an u64.
> 
> Signed-off-by: huijin.park 
> Reviewed-by: Geert Uytterhoeven 

Applied to http://git.infradead.org/linux-mtd.git mtd/next, thanks.

Boris


Re: mtd: use DEFINE_SHOW_ATTRIBUTE() instead of open-coding it

2018-12-02 Thread Boris Brezillon
On Sun, 2018-12-02 at 08:33:58 UTC, Yangtao Li wrote:
> DEFINE_SHOW_ATTRIBUTE macro can help us simplify the code,so change
> to it.And change the DEBUGFS_RO_ATTR macro defined in some file to a
> standard macro.
> 
> Signed-off-by: Yangtao Li 

Applied to http://git.infradead.org/linux-mtd.git mtd/next, thanks.

Boris


Re: [PATCH] mtd: use DEFINE_SHOW_ATTRIBUTE() instead of open-coding it

2018-12-02 Thread Boris Brezillon
Should be [PATCH v4]

On Sun,  2 Dec 2018 03:33:58 -0500
Yangtao Li  wrote:

> DEFINE_SHOW_ATTRIBUTE macro can help us simplify the code,so change
> to it.And change the DEBUGFS_RO_ATTR macro defined in some file to a
> standard macro.

You still don't put spaces after commas or periods

> 
> Signed-off-by: Yangtao Li 
> ---

and you miss a changelog here.

Anyway, no need to send a new version, I'll fix the commit message when
applying the patch.

Thanks,

Boris

>  drivers/mtd/devices/docg3.c| 16 
>  drivers/mtd/devices/docg3.h| 11 ---
>  drivers/mtd/mtdswap.c  | 13 +
>  drivers/mtd/nand/raw/nandsim.c | 17 +++--
>  4 files changed, 12 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
> index 512bd4c2eec0..4c94fc096696 100644
> --- a/drivers/mtd/devices/docg3.c
> +++ b/drivers/mtd/devices/docg3.c
> @@ -1603,7 +1603,7 @@ static void doc_unregister_sysfs(struct platform_device 
> *pdev,
>  /*
>   * Debug sysfs entries
>   */
> -static int dbg_flashctrl_show(struct seq_file *s, void *p)
> +static int flashcontrol_show(struct seq_file *s, void *p)
>  {
>   struct docg3 *docg3 = (struct docg3 *)s->private;
>  
> @@ -1623,9 +1623,9 @@ static int dbg_flashctrl_show(struct seq_file *s, void 
> *p)
>  
>   return 0;
>  }
> -DEBUGFS_RO_ATTR(flashcontrol, dbg_flashctrl_show);
> +DEFINE_SHOW_ATTRIBUTE(flashcontrol);
>  
> -static int dbg_asicmode_show(struct seq_file *s, void *p)
> +static int asic_mode_show(struct seq_file *s, void *p)
>  {
>   struct docg3 *docg3 = (struct docg3 *)s->private;
>  
> @@ -1660,9 +1660,9 @@ static int dbg_asicmode_show(struct seq_file *s, void 
> *p)
>   seq_puts(s, ")\n");
>   return 0;
>  }
> -DEBUGFS_RO_ATTR(asic_mode, dbg_asicmode_show);
> +DEFINE_SHOW_ATTRIBUTE(asic_mode);
>  
> -static int dbg_device_id_show(struct seq_file *s, void *p)
> +static int device_id_show(struct seq_file *s, void *p)
>  {
>   struct docg3 *docg3 = (struct docg3 *)s->private;
>   int id;
> @@ -1674,9 +1674,9 @@ static int dbg_device_id_show(struct seq_file *s, void 
> *p)
>   seq_printf(s, "DeviceId = %d\n", id);
>   return 0;
>  }
> -DEBUGFS_RO_ATTR(device_id, dbg_device_id_show);
> +DEFINE_SHOW_ATTRIBUTE(device_id);
>  
> -static int dbg_protection_show(struct seq_file *s, void *p)
> +static int protection_show(struct seq_file *s, void *p)
>  {
>   struct docg3 *docg3 = (struct docg3 *)s->private;
>   int protect, dps0, dps0_low, dps0_high, dps1, dps1_low, dps1_high;
> @@ -1726,7 +1726,7 @@ static int dbg_protection_show(struct seq_file *s, void 
> *p)
>  !!(dps1 & DOC_DPS_KEY_OK));
>   return 0;
>  }
> -DEBUGFS_RO_ATTR(protection, dbg_protection_show);
> +DEFINE_SHOW_ATTRIBUTE(protection);
>  
>  static void __init doc_dbg_register(struct mtd_info *floor)
>  {
> diff --git a/drivers/mtd/devices/docg3.h b/drivers/mtd/devices/docg3.h
> index e99946575398..e16dca23655b 100644
> --- a/drivers/mtd/devices/docg3.h
> +++ b/drivers/mtd/devices/docg3.h
> @@ -317,17 +317,6 @@ struct docg3 {
>  #define doc_info(fmt, arg...) dev_info(docg3->dev, (fmt), ## arg)
>  #define doc_dbg(fmt, arg...) dev_dbg(docg3->dev, (fmt), ## arg)
>  #define doc_vdbg(fmt, arg...) dev_vdbg(docg3->dev, (fmt), ## arg)
> -
> -#define DEBUGFS_RO_ATTR(name, show_fct) \
> - static int name##_open(struct inode *inode, struct file *file) \
> - { return single_open(file, show_fct, inode->i_private); }  \
> - static const struct file_operations name##_fops = { \
> - .owner = THIS_MODULE, \
> - .open = name##_open, \
> - .llseek = seq_lseek, \
> - .read = seq_read, \
> - .release = single_release \
> - };
>  #endif
>  
>  /*
> diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
> index d9dcb2d051b4..d162d1717fad 100644
> --- a/drivers/mtd/mtdswap.c
> +++ b/drivers/mtd/mtdswap.c
> @@ -1265,18 +1265,7 @@ static int mtdswap_show(struct seq_file *s, void *data)
>  
>   return 0;
>  }
> -
> -static int mtdswap_open(struct inode *inode, struct file *file)
> -{
> - return single_open(file, mtdswap_show, inode->i_private);
> -}
> -
> -static const struct file_operations mtdswap_fops = {
> - .open   = mtdswap_open,
> - .read   = seq_read,
> - .llseek = seq_lseek,
> - .release= single_release,
> -};
> +DEFINE_SHOW_ATTRIBUTE(mtdswap);
>  
>  static int mtdswap_add_debugfs(struct mtdswap_dev *d)
>  {
> diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
> index c452819f6123..ef8721418f2d 100644
> --- a/drivers/mtd/nand/raw/nandsim.c
> +++ b/drivers/mtd/nand/raw/nandsim.c
> @@ -443,7 +443,7 @@ static unsigned long total_wear = 0;
>  /* MTD structure for NAND controller */
>  static struct mtd_info *nsmtd;
>  
> -static int nandsim_debugfs_show(struct seq_file *m, void *private)
> +static int nandsim_show(struct 

Re: [PATCH v3] mtd: remove DEBUGFS_RO_ATTR()

2018-12-02 Thread Boris Brezillon
Looks like getting rid of DEBUGFS_RO_ATTR() is just one of the change
you do. I think you should change the subject line:

"mtd: use DEFINE_SHOW_ATTRIBUTE() instead of open-coding it"

This way it covers all of your changes.

On Sun,  2 Dec 2018 02:32:02 -0500
Yangtao Li  wrote:

> We already have the DEFINE_SHOW_ATTRIBUTE.There is no need to define

^ missing space

> such a macro,so remove DEBUGFS_RO_ATTR.Also use DEFINE_SHOW_ATTRIBUTE

   ^ here as well

> to simplify some code.

If you go for the new subject line, you'll have to change this commit
message.

> 
> Signed-off-by: Yangtao Li 
> ---
> changes in v3:
> -remove the blank line between the function
>  definition and DEFINE_SHOW_ATTRIBUTE()
> ---
>  drivers/mtd/devices/docg3.c| 16 
>  drivers/mtd/devices/docg3.h| 11 ---
>  drivers/mtd/mtdswap.c  | 13 +
>  drivers/mtd/nand/raw/nandsim.c | 17 +++--
>  4 files changed, 12 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
> index 512bd4c2eec0..4c94fc096696 100644
> --- a/drivers/mtd/devices/docg3.c
> +++ b/drivers/mtd/devices/docg3.c
> @@ -1603,7 +1603,7 @@ static void doc_unregister_sysfs(struct platform_device 
> *pdev,
>  /*
>   * Debug sysfs entries
>   */
> -static int dbg_flashctrl_show(struct seq_file *s, void *p)
> +static int flashcontrol_show(struct seq_file *s, void *p)
>  {
>   struct docg3 *docg3 = (struct docg3 *)s->private;
>  
> @@ -1623,9 +1623,9 @@ static int dbg_flashctrl_show(struct seq_file *s, void 
> *p)
>  
>   return 0;
>  }
> -DEBUGFS_RO_ATTR(flashcontrol, dbg_flashctrl_show);
> +DEFINE_SHOW_ATTRIBUTE(flashcontrol);
>  
> -static int dbg_asicmode_show(struct seq_file *s, void *p)
> +static int asic_mode_show(struct seq_file *s, void *p)
>  {
>   struct docg3 *docg3 = (struct docg3 *)s->private;
>  
> @@ -1660,9 +1660,9 @@ static int dbg_asicmode_show(struct seq_file *s, void 
> *p)
>   seq_puts(s, ")\n");
>   return 0;
>  }
> -DEBUGFS_RO_ATTR(asic_mode, dbg_asicmode_show);
> +DEFINE_SHOW_ATTRIBUTE(asic_mode);
>  
> -static int dbg_device_id_show(struct seq_file *s, void *p)
> +static int device_id_show(struct seq_file *s, void *p)
>  {
>   struct docg3 *docg3 = (struct docg3 *)s->private;
>   int id;
> @@ -1674,9 +1674,9 @@ static int dbg_device_id_show(struct seq_file *s, void 
> *p)
>   seq_printf(s, "DeviceId = %d\n", id);
>   return 0;
>  }
> -DEBUGFS_RO_ATTR(device_id, dbg_device_id_show);
> +DEFINE_SHOW_ATTRIBUTE(device_id);
>  
> -static int dbg_protection_show(struct seq_file *s, void *p)
> +static int protection_show(struct seq_file *s, void *p)
>  {
>   struct docg3 *docg3 = (struct docg3 *)s->private;
>   int protect, dps0, dps0_low, dps0_high, dps1, dps1_low, dps1_high;
> @@ -1726,7 +1726,7 @@ static int dbg_protection_show(struct seq_file *s, void 
> *p)
>  !!(dps1 & DOC_DPS_KEY_OK));
>   return 0;
>  }
> -DEBUGFS_RO_ATTR(protection, dbg_protection_show);
> +DEFINE_SHOW_ATTRIBUTE(protection);
>  
>  static void __init doc_dbg_register(struct mtd_info *floor)
>  {
> diff --git a/drivers/mtd/devices/docg3.h b/drivers/mtd/devices/docg3.h
> index e99946575398..e16dca23655b 100644
> --- a/drivers/mtd/devices/docg3.h
> +++ b/drivers/mtd/devices/docg3.h
> @@ -317,17 +317,6 @@ struct docg3 {
>  #define doc_info(fmt, arg...) dev_info(docg3->dev, (fmt), ## arg)
>  #define doc_dbg(fmt, arg...) dev_dbg(docg3->dev, (fmt), ## arg)
>  #define doc_vdbg(fmt, arg...) dev_vdbg(docg3->dev, (fmt), ## arg)
> -
> -#define DEBUGFS_RO_ATTR(name, show_fct) \
> - static int name##_open(struct inode *inode, struct file *file) \
> - { return single_open(file, show_fct, inode->i_private); }  \
> - static const struct file_operations name##_fops = { \
> - .owner = THIS_MODULE, \
> - .open = name##_open, \
> - .llseek = seq_lseek, \
> - .read = seq_read, \
> - .release = single_release \
> - };
>  #endif
>  
>  /*
> diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
> index d9dcb2d051b4..d162d1717fad 100644
> --- a/drivers/mtd/mtdswap.c
> +++ b/drivers/mtd/mtdswap.c
> @@ -1265,18 +1265,7 @@ static int mtdswap_show(struct seq_file *s, void *data)
>  
>   return 0;
>  }
> -
> -static int mtdswap_open(struct inode *inode, struct file *file)
> -{
> - return single_open(file, mtdswap_show, inode->i_private);
> -}
> -
> -static const struct file_operations mtdswap_fops = {
> - .open   = mtdswap_open,
> - .read   = seq_read,
> - .llseek = seq_lseek,
> - .release= single_release,
> -};
> +DEFINE_SHOW_ATTRIBUTE(mtdswap);
>  
>  static int mtdswap_add_debugfs(struct mtdswap_dev *d)
>  {
> diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
> index c452819f6123..ef8721418f2d 100644
> --- 

Re: [PATCH v2] mtd: remove DEBUGFS_RO_ATTR()

2018-12-01 Thread Boris Brezillon
On Sat,  1 Dec 2018 20:54:17 -0500
Yangtao Li  wrote:

> We already have the DEFINE_SHOW_ATTRIBUTE.There is no need to define
> such a macro,so remove DEBUGFS_RO_ATTR.Also use DEFINE_SHOW_ATTRIBUTE
> to simplify some code.
> 
> Signed-off-by: Yangtao Li 
> ---
> Changes in v2:
> -Remove a missing DEBUGFS_RO_ATTR
> ---
>  drivers/mtd/devices/docg3.c| 20 
>  drivers/mtd/devices/docg3.h| 11 ---
>  drivers/mtd/mtdswap.c  | 12 +---
>  drivers/mtd/nand/raw/nandsim.c | 16 +++-
>  4 files changed, 16 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
> index 512bd4c2eec0..80143972963e 100644
> --- a/drivers/mtd/devices/docg3.c
> +++ b/drivers/mtd/devices/docg3.c
> @@ -1603,7 +1603,7 @@ static void doc_unregister_sysfs(struct platform_device 
> *pdev,
>  /*
>   * Debug sysfs entries
>   */
> -static int dbg_flashctrl_show(struct seq_file *s, void *p)
> +static int flashcontrol_show(struct seq_file *s, void *p)
>  {
>   struct docg3 *docg3 = (struct docg3 *)s->private;
>  
> @@ -1623,9 +1623,10 @@ static int dbg_flashctrl_show(struct seq_file *s, void 
> *p)
>  
>   return 0;
>  }
> -DEBUGFS_RO_ATTR(flashcontrol, dbg_flashctrl_show);
>  
> -static int dbg_asicmode_show(struct seq_file *s, void *p)
> +DEFINE_SHOW_ATTRIBUTE(flashcontrol);

Just nitpicking, but can you remove the blank line between the function
definition and DEFINE_SHOW_ATTRIBUTE()?


Re: [PATCH RFC 09/15] mtd: replace **** with a hug

2018-11-30 Thread Boris Brezillon
On Fri, 30 Nov 2018 11:27:18 -0800
Jarkko Sakkinen  wrote:

> In order to comply with the CoC, replace  with a hug.
> 
> Signed-off-by: Jarkko Sakkinen 
> ---
>  drivers/mtd/mtd_blkdevs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> index b0d44f9214b0..bdf68678fccc 100644
> --- a/drivers/mtd/mtd_blkdevs.c
> +++ b/drivers/mtd/mtd_blkdevs.c
> @@ -565,7 +565,7 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
>   int ret;
>  
>   /* Register the notifier if/when the first device type is
> -registered, to prevent the link/init ordering from fucking
> +registered, to prevent the link/init ordering from hugging
>  us over. */

Please rephrase the comment instead of this replacement.

>   if (!blktrans_notifier.list.next)
>   register_mtd_user(_notifier);



[GIT PULL] mtd: Fixes for 4.20-rc5

2018-11-30 Thread Boris Brezillon
Hello Linus,

Here is the MTD fixes PR for 4.20-rc5.

Regards,

Boris

The following changes since commit 2e6e902d185027f8e3cb8b7305238f7e35d6a436:

  Linux 4.20-rc4 (2018-11-25 14:19:31 -0800)

are available in the Git repository at:

  git://git.infradead.org/linux-mtd.git tags/mtd/fixes-for-4.20-rc5

for you to fetch changes up to 40b412897ccb4b98b2cfb2a0aaabed58dd9e2086:

  mtd: nand: Fix memory allocation in nanddev_bbt_init() (2018-11-28 15:41:50 
+0100)


NAND fixes:
- Fix BBT cache allocation done in nanddev_bbt_init()

SPI NOR fixes:
- Fix the erase type selection logic


Frieder Schrempf (1):
  mtd: nand: Fix memory allocation in nanddev_bbt_init()

Tudor Ambarus (1):
  mtd: spi-nor: fix erase_type array to indicate current map conf

 drivers/mtd/nand/bbt.c|  3 ++-
 drivers/mtd/spi-nor/spi-nor.c | 31 +--
 2 files changed, 31 insertions(+), 3 deletions(-)  


Re: mtd: nand: Fix memory allocation in nanddev_bbt_init()

2018-11-30 Thread Boris Brezillon
On Tue, 2018-11-27 at 07:44:52 UTC, Schrempf Frieder wrote:
> Fix the size of the buffer allocated to store the in-memory BBT.
> This bug was previously hidden by a different bug, that was fixed in
> d098093ba06e.
> 
> Fixes: 9c3736a3de21 ("mtd: nand: Add core infrastructure to deal with NAND 
> devices")
> Cc: 
> Signed-off-by: Frieder Schrempf 
> Acked-by: Miquel Raynal 

Applied to http://git.infradead.org/linux-mtd.git master, thanks.

Boris


Re: [v2] mtd: spi-nor: fix erase_type array to indicate current map conf

2018-11-30 Thread Boris Brezillon
On Mon, 2018-11-26 at 12:45:44 UTC,  wrote:
> From: Tudor Ambarus 
> 
> BFPT advertises all the erase types supported by all the possible
> map configurations. Mask out the erase types that are not supported
> by the current map configuration.
> 
> Backward compatibility test done on sst26vf064b.
> 
> Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> Reported-by: Alexander Sverdlin 
> Signed-off-by: Tudor Ambarus 
> Tested-by: Alexander Sverdlin 

Applied to http://git.infradead.org/linux-mtd.git master, thanks.

Boris


Re: [PATCH v2 01/25] nvmem: add new config option

2018-11-29 Thread Boris Brezillon
On Thu, 29 Nov 2018 15:35:58 +0100
Bartosz Golaszewski  wrote:

> czw., 29 lis 2018 o 15:32 Srinivas Kandagatla
>  napisał(a):
> >
> >  
> > >
> > > Hi Srinivas,
> > >
> > > if there are no objections - can you Ack this patch for Greg to pick
> > > up into char-misc?  
> >
> > Patch is fine for me. I normally send all the nvmem patches just before rc5.
> >
> > --srini  
> > >
> > > Bart
> > >  
> 
> Boris, are you fine with Srinivas sending the MTD patch as well so
> that they stay together?

Yes, I'll just need an immutable branch/tag in case we have conflicting
changes in the MTD tree.


Re: [PATCH] mtd: nand: Fix memory allocation in nanddev_bbt_init()

2018-11-28 Thread Boris Brezillon
On Wed, 28 Nov 2018 15:19:37 +
Schrempf Frieder  wrote:

> On 28.11.18 16:02, Boris Brezillon wrote:
> > On Wed, 28 Nov 2018 14:55:45 +
> > Schrempf Frieder  wrote:
> >   
> >> Hi Boris,
> >>
> >> On 28.11.18 15:41, Boris Brezillon wrote:  
> >>> On Tue, 27 Nov 2018 07:44:52 +
> >>> Schrempf Frieder  wrote:
> >>>  
> >>>> Fix the size of the buffer allocated to store the in-memory BBT.
> >>>> This bug was previously hidden by a different bug, that was fixed in
> >>>> d098093ba06e.
> >>>>
> >>>> Fixes: 9c3736a3de21 ("mtd: nand: Add core infrastructure to deal with 
> >>>> NAND devices")
> >>>> Cc: 
> >>>> Signed-off-by: Frieder Schrempf   
> >>>
> >>> Looks like your From header does not match the SoB tag
> >>> ('Frieder Schrempf' vs 'Schrempf Frieder') and checkpatch does not like
> >>> that. I'll fix it when applying, but maybe you should fix
> >>> your .gitconfig to make them match.  
> >>
> >> Actually the From header in my local patch is correct (
> >> , Frieder Schrempf) as it comes from my git config. But since
> >> our company was renamed and our mail servers were transferred, our
> >> e-mails are sent with From= .
> >>
> >> It seems like git send-email or patchwork or whatever uses the
> >> information from the e-mail header instead of what is in the patch.
> >>
> >> I will try to raise this issue with our IT department as this would be
> >> best fixed on their side.  
> > 
> > There's another solution: make git send-email add a From header in the
> > message body.
> > 
> > git config --global sendemail.from "Schrempf Frieder 
> > "  
> 
> I don't get it. How would that change things? My From still wouldn't 
> match my SoB tags.

When there's a From line in the message Body, git am will use this one
instead of the one in the header section.


Re: [PATCH] mtd: nand: Fix memory allocation in nanddev_bbt_init()

2018-11-28 Thread Boris Brezillon
On Wed, 28 Nov 2018 14:55:45 +
Schrempf Frieder  wrote:

> Hi Boris,
> 
> On 28.11.18 15:41, Boris Brezillon wrote:
> > On Tue, 27 Nov 2018 07:44:52 +
> > Schrempf Frieder  wrote:
> >   
> >> Fix the size of the buffer allocated to store the in-memory BBT.
> >> This bug was previously hidden by a different bug, that was fixed in
> >> d098093ba06e.
> >>
> >> Fixes: 9c3736a3de21 ("mtd: nand: Add core infrastructure to deal with NAND 
> >> devices")
> >> Cc: 
> >> Signed-off-by: Frieder Schrempf   
> > 
> > Looks like your From header does not match the SoB tag
> > ('Frieder Schrempf' vs 'Schrempf Frieder') and checkpatch does not like
> > that. I'll fix it when applying, but maybe you should fix
> > your .gitconfig to make them match.  
> 
> Actually the From header in my local patch is correct ( 
> , Frieder Schrempf) as it comes from my git config. But since 
> our company was renamed and our mail servers were transferred, our 
> e-mails are sent with From= .
> 
> It seems like git send-email or patchwork or whatever uses the 
> information from the e-mail header instead of what is in the patch.
> 
> I will try to raise this issue with our IT department as this would be 
> best fixed on their side.

There's another solution: make git send-email add a From header in the
message body.

git config --global sendemail.from "Schrempf Frieder 
"


Re: [PATCH] mtd: nand: Fix memory allocation in nanddev_bbt_init()

2018-11-28 Thread Boris Brezillon
On Tue, 27 Nov 2018 07:44:52 +
Schrempf Frieder  wrote:

> Fix the size of the buffer allocated to store the in-memory BBT.
> This bug was previously hidden by a different bug, that was fixed in
> d098093ba06e.
> 
> Fixes: 9c3736a3de21 ("mtd: nand: Add core infrastructure to deal with NAND 
> devices")
> Cc: 
> Signed-off-by: Frieder Schrempf 

Looks like your From header does not match the SoB tag
('Frieder Schrempf' vs 'Schrempf Frieder') and checkpatch does not like
that. I'll fix it when applying, but maybe you should fix
your .gitconfig to make them match.

> ---
>  drivers/mtd/nand/bbt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/bbt.c b/drivers/mtd/nand/bbt.c
> index 56cde38..c12497f 100644
> --- a/drivers/mtd/nand/bbt.c
> +++ b/drivers/mtd/nand/bbt.c
> @@ -27,7 +27,8 @@ int nanddev_bbt_init(struct nand_device *nand)
>   unsigned int nwords = DIV_ROUND_UP(nblocks * bits_per_block,
>  BITS_PER_LONG);
>  
> - nand->bbt.cache = kzalloc(nwords, GFP_KERNEL);
> + nand->bbt.cache = kzalloc(nwords * (BITS_PER_LONG / BITS_PER_BYTE),
> +   GFP_KERNEL);
>   if (!nand->bbt.cache)
>   return -ENOMEM;
>  



Re: [PATCH] mtd: spi-nor: parse SFDP 4-byte Address Instruction Table

2018-11-28 Thread Boris Brezillon
On Wed, 28 Nov 2018 14:17:12 +
 wrote:

> On 11/28/2018 09:57 AM, Boris Brezillon wrote:
> > On Tue, 20 Nov 2018 11:55:21 +
> >  wrote:
> >   
> >> +
> >> +  /*
> >> +   * We set nor->addr_width here to skip spi_nor_set_4byte_opcodes()
> >> +   * later because this latest function implements a legacy quirk for
> >> +   * the erase size of Spansion memory. However this quirk is no longer
> >> +   * needed with new SFDP compliant memories.
> >> +   */
> >> +  nor->addr_width = 4;
> >> +  nor->flags |= SPI_NOR_4B_OPCODES;  
> > 
> > You mean SNOR_F_4B_OPCODES (the one introduced here [1]), because
> > SPI_NOR_4B_OPCODES should only be used for flash_info->flags and might
> > soon conflict with another SNOR_F_ flag?
> >   
> 
> yes, you're right.
> 
> > [1]http://patchwork.ozlabs.org/patch/991476/
> >   
> 
> Can you apply your patch? Will submit a new version afterwards.

Actually, I realized setting SNOR_F_4B_OPCODES when the BFPT advertises
4_BYTES_ONLY is incorrect as 4bytes only can mean "use the 3B opcodes
but pass address on 4 bytes". Here is a new version of this patch [1].
Feel free to pick it up and send it along with your "SFDP 4-byte Address
Instruction Table" patch (I have not reason to send it alone since the
problem I was trying to solve is no longer fixed by [1]).

[1]https://github.com/bbrezillon/linux-0day/commit/a953b6b435ec67bca00df472db5f6dca4f63


Re: [PATCH v2] mtd: spi-nor: fix erase_type array to indicate current map conf

2018-11-28 Thread Boris Brezillon
Hi Alexander,

On Mon, 26 Nov 2018 13:04:45 +
"Sverdlin, Alexander (Nokia - DE/Ulm)" 
wrote:

> Hello Tudor and all,
> 
> On 26/11/2018 13:45, tudor.amba...@microchip.com wrote:
> > From: Tudor Ambarus 
> > 
> > BFPT advertises all the erase types supported by all the possible
> > map configurations. Mask out the erase types that are not supported
> > by the current map configuration.
> > 
> > Backward compatibility test done on sst26vf064b.
> > 
> > Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> > Reported-by: Alexander Sverdlin   
> 
> I've verified the erasesize, partitioning and erase function of the S25FS128S
> and all the above is at least back to pre-SFDP state.
> 
> So the patch works as intended and it's
> Tested-by: Alexander Sverdlin 
> 
> There is still a regression with write when S25FS128S is used with SFDP
> because SFDP of the S25FS128S is just broken. It advertises 512 bytes
> of write page size but factory delivery configuration wraps the address
> on 256 bytes boundary. I found no way to fetch this configuration in a
> vendor-neutral way (or JEDEC-conform way). Which means, S25FS128S
> could be the first user of SPI_NOR_SKIP_SFDP flag :\.

You might be interested in my work around SPI NOR fixups[1]. I added a
few fixup hooks that can be implemented by manufacturer drivers (a new
concept) or on a per-chip basis. Looks like you'd need to implement a
->post_bfpt() fixup hook for this particular chip (see what's been done
for this Macronix chip [2]).

Regards,

Boris

[1]https://github.com/bbrezillon/linux-0day/commits/spi-nor/manuf-drv
[2]https://github.com/bbrezillon/linux-0day/commit/b29625fdc55fa8ccfd4299904d727b44f8382c18


Re: [PATCH] mtd: nand: spi: Add initial support for Toshiba TC58CVG2S0H

2018-11-28 Thread Boris Brezillon
On Tue, 27 Nov 2018 16:41:56 +
Schrempf Frieder  wrote:

> >>> +static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
> >>> +  struct mtd_oob_region *region)
> >>> +{
> >>> + if (section > 7)
> >>> + return -ERANGE;
> >>> +
> >>> + region->offset = 128 + 16 * section;
> >>> + region->length = 16;  
> > 
> > Here you expose the ECC bits has 8 sections of 16 bytes.
> > But regarding the datasheet this should not be accessed page 32.
> > "The ECC parity code generated by internal ECC is stored in column
> > addresses 4224-4351 and the users cannot access to these specific
> > addresses when internal ECC is enabled."  

'when internal ECC is enabled' means that those bytes can be accessed
when it's disabled. We should keep exposing the ECC byte sections. Note
that even if ECC sections are not exposed, the core will read those
bytes. They're probably filled with garbage in this case, but we don't
care since we won't use them.

> 
> This is just to let the other layers know, where the bytes used for ECC 
> are. As long as the driver uses the on-chip ECC it won't write to this 
> area. So this is correct unless I misunderstood this concept. All the 
> other supported SPI NAND chips use the same approach.

Yes, and that's the right thing to do. We want to know where the ECC
bytes are, especially when doing raw accesses.

> 
> >   
> >>> +
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
> >>> +   struct mtd_oob_region *region)
> >>> +{
> >>> + if (section > 7)
> >>> + return -ERANGE;
> >>> +
> >>> + region->offset = 2 + 16 * section;
> >>> + region->length = 14;  
> > 
> > This reserved 2 bytes for BBM for each section.
> > But maybe we could declare this as 1 section of 128bytes:
> > 
> > if (section)
> >   return -ERANGE;
> > 
> > region->offset = 2;
> > region->length = 126;  

I agree with this suggestion: if the free bytes are contiguous, it's
better to expose them as a single section.


Re: [PATCH] ubi: do not drop UBI device reference before using

2018-11-28 Thread Boris Brezillon
On Wed, 28 Nov 2018 11:20:03 +0800
Pan Bian  wrote:

> The UBI device reference is dropped but then the device is used as a
> parameter of ubi_err. The bug is introduced in changing ubi_err's 
> behavior. The old ubi_err does not require a UBI device as its first 
> parameter, but the new one does.
> 
> Fixes: 32608703310 ("UBI: Extend UBI layer debug/messaging capabilities")
> 

Unnecessary blank line here.

> Signed-off-by: Pan Bian 

Reviewed-by: Boris Brezillon 

> ---
>  drivers/mtd/ubi/kapi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/ubi/kapi.c b/drivers/mtd/ubi/kapi.c
> index e9e9ecb..0b8f0c4 100644
> --- a/drivers/mtd/ubi/kapi.c
> +++ b/drivers/mtd/ubi/kapi.c
> @@ -227,9 +227,9 @@ struct ubi_volume_desc *ubi_open_volume(int ubi_num, int 
> vol_id, int mode)
>  out_free:
>   kfree(desc);
>  out_put_ubi:
> - ubi_put_device(ubi);
>   ubi_err(ubi, "cannot open device %d, volume %d, error %d",
>   ubi_num, vol_id, err);
> + ubi_put_device(ubi);
>   return ERR_PTR(err);
>  }
>  EXPORT_SYMBOL_GPL(ubi_open_volume);



Re: [PATCH] ubi: put MTD device after it is not used

2018-11-28 Thread Boris Brezillon
On Wed, 28 Nov 2018 10:57:33 +0800
Pan Bian  wrote:

> The MTD device reference is dropped via put_mtd_device, however its
> field ->index is read and passed to ubi_msg. To fix this, the patch
> moves the reference dropping after calling ubi_msg.
> 
> Signed-off-by: Pan Bian 

Reviewed-by: Boris Brezillon 

> ---
>  drivers/mtd/ubi/build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index a4e3454..09170b7 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -1101,10 +1101,10 @@ int ubi_detach_mtd_dev(int ubi_num, int anyway)
>   ubi_wl_close(ubi);
>   ubi_free_internal_volumes(ubi);
>   vfree(ubi->vtbl);
> - put_mtd_device(ubi->mtd);
>   vfree(ubi->peb_buf);
>   vfree(ubi->fm_buf);
>   ubi_msg(ubi, "mtd%d is detached", ubi->mtd->index);
> + put_mtd_device(ubi->mtd);

Maybe we should move some of these to the dev->release() or
class->dev_release() hook.

>   put_device(>dev);
>   return 0;
>  }



Re: [PATCH] mtd: spi-nor: parse SFDP 4-byte Address Instruction Table

2018-11-27 Thread Boris Brezillon
On Tue, 20 Nov 2018 11:55:21 +
 wrote:

> +
> + /*
> +  * We set nor->addr_width here to skip spi_nor_set_4byte_opcodes()
> +  * later because this latest function implements a legacy quirk for
> +  * the erase size of Spansion memory. However this quirk is no longer
> +  * needed with new SFDP compliant memories.
> +  */
> + nor->addr_width = 4;
> + nor->flags |= SPI_NOR_4B_OPCODES;

You mean SNOR_F_4B_OPCODES (the one introduced here [1]), because
SPI_NOR_4B_OPCODES should only be used for flash_info->flags and might
soon conflict with another SNOR_F_ flag?

[1]http://patchwork.ozlabs.org/patch/991476/


Re: [PATCH] mtd: nand: Fix memory allocation in nanddev_bbt_init()

2018-11-27 Thread Boris Brezillon
On Tue, 27 Nov 2018 07:44:52 +
Schrempf Frieder  wrote:

> Fix the size of the buffer allocated to store the in-memory BBT.
> This bug was previously hidden by a different bug, that was fixed in
> d098093ba06e.

Oops :-/.

> 
> Fixes: 9c3736a3de21 ("mtd: nand: Add core infrastructure to deal with NAND 
> devices")
> Cc: 
> Signed-off-by: Frieder Schrempf 
> ---
>  drivers/mtd/nand/bbt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/bbt.c b/drivers/mtd/nand/bbt.c
> index 56cde38..c12497f 100644
> --- a/drivers/mtd/nand/bbt.c
> +++ b/drivers/mtd/nand/bbt.c
> @@ -27,7 +27,8 @@ int nanddev_bbt_init(struct nand_device *nand)
>   unsigned int nwords = DIV_ROUND_UP(nblocks * bits_per_block,
>  BITS_PER_LONG);
>  
> - nand->bbt.cache = kzalloc(nwords, GFP_KERNEL);
> + nand->bbt.cache = kzalloc(nwords * (BITS_PER_LONG / BITS_PER_BYTE),

I prefer

 * sizeof(*nand->bbt.cache)
If you're okay with this change, I'll fix it when applying (no need to
send a new version).

Thanks,

Boris

> +   GFP_KERNEL);
>   if (!nand->bbt.cache)
>   return -ENOMEM;
>  



Re: [PATCH 4.14 58/62] mtd: rawnand: atmel: fix OF child-node lookup

2018-11-26 Thread Boris Brezillon
On Mon, 26 Nov 2018 19:46:15 +0530
Naresh Kamboju  wrote:

> Do you see build failure arm x15 beagleboard on 4.14 due to this patch ?
> 
> On Mon, 26 Nov 2018 at 16:31, Greg Kroah-Hartman
>  wrote:
> >
> > 4.14-stable review patch.  If anyone has any objections, please let me know.
> >
> > --
> >
> > From: Johan Hovold 
> >
> > commit 5d1e9c2212ea6b4dd735e4fc3dd6279a365d5d10 upstream.
> >
> > Use the new of_get_compatible_child() helper to lookup the nfc child
> > node instead of using of_find_compatible_node(), which searches the
> > entire tree from a given start node and thus can return an unrelated
> > (i.e. non-child) node.
> >
> > This also addresses a potential use-after-free (e.g. after probe
> > deferral) as the tree-wide helper drops a reference to its first
> > argument (i.e. the node of the device being probed).
> >
> > While at it, also fix a related nfc-node reference leak.
> >
> > Fixes: f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")
> > Cc: stable  # 4.11
> > Cc: Nicolas Ferre 
> > Cc: Josh Wu 
> > Cc: Boris Brezillon 
> > Signed-off-by: Johan Hovold 
> > Signed-off-by: Boris Brezillon 
> > Signed-off-by: Greg Kroah-Hartman 
> >
> > ---
> >  drivers/mtd/nand/atmel/nand-controller.c |   11 +++
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > --- a/drivers/mtd/nand/atmel/nand-controller.c
> > +++ b/drivers/mtd/nand/atmel/nand-controller.c
> > @@ -2077,8 +2077,7 @@ atmel_hsmc_nand_controller_legacy_init(s
> > int ret;
> >
> > nand_np = dev->of_node;
> > -   nfc_np = of_find_compatible_node(dev->of_node, NULL,
> > -"atmel,sama5d3-nfc");
> > +   nfc_np = of_get_compatible_child(dev->of_node, "atmel,sama5d3-nfc");
> > if (!nfc_np) {
> > dev_err(dev, "Could not find device node for 
> > sama5d3-nfc\n");
> > return -ENODEV;
> > @@ -2492,15 +2491,19 @@ static int atmel_nand_controller_probe(s
> > }
> >
> > if (caps->legacy_of_bindings) {
> > +   struct device_node *nfc_node;
> > u32 ale_offs = 21;
> >
> > /*
> >  * If we are parsing legacy DT props and the DT contains a
> >  * valid NFC node, forward the request to the sama5 logic.
> >  */
> > -   if (of_find_compatible_node(pdev->dev.of_node, NULL,
> > -   "atmel,sama5d3-nfc"))
> > +   nfc_node = of_get_compatible_child(pdev->dev.of_node,
> > +  "atmel,sama5d3-nfc");
> > +   if (nfc_node) {
> > caps = _sama5_nand_caps;
> > +   of_node_put(nfc_node);
> > +   }
> >
> > /*
> >  * Even if the compatible says we are dealing with an
> >
> >  
> 
> /drivers/mtd/nand/atmel/nand-controller.c: In function
> 'atmel_hsmc_nand_controller_legacy_init':
> /drivers/mtd/nand/atmel/nand-controller.c:2080:11: error: implicit
> declaration of function 'of_get_compatible_child'; did you mean
> 'of_get_next_available_child'? [-Werror=implicit-function-declaration]
>nfc_np = of_get_compatible_child(dev->of_node, "atmel,sama5d3-nfc");

Looks like of_get_compatible_child() has been introduced in 4.18, hence
this error.

Greg, can you drop this patch from 4.14.y?


Re: [PATCH 2/2] ic3: master: off by one in mode_show()

2018-11-26 Thread Boris Brezillon
On Fri, 23 Nov 2018 10:15:05 +0300
Dan Carpenter  wrote:

> This should be >= ARRAY_SIZE() to avoid reading one element beyond the
> end of the array.
> 
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Signed-off-by: Dan Carpenter 

Queued to i3c/next.

Thanks,

Boris

> ---
>  drivers/i3c/master.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index bda4b9613e53..c39f89d2deba 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -475,7 +475,7 @@ static ssize_t mode_show(struct device *dev,
>  
>   i3c_bus_normaluse_lock(i3cbus);
>   if (i3cbus->mode < 0 ||
> - i3cbus->mode > ARRAY_SIZE(i3c_bus_mode_strings) ||
> + i3cbus->mode >= ARRAY_SIZE(i3c_bus_mode_strings) ||
>   !i3c_bus_mode_strings[i3cbus->mode])
>   ret = sprintf(buf, "unknown\n");
>   else



Re: [PATCH 1/2] i3c: fix an error code in i3c_master_add_i3c_dev_locked()

2018-11-26 Thread Boris Brezillon
On Fri, 23 Nov 2018 10:14:42 +0300
Dan Carpenter  wrote:

> We should return "ret" as-is.  The "newdev" variable is a valid pointer.
> 
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Signed-off-by: Dan Carpenter 

Queued to i3c/next.

Thanks,

Boris

> ---
>  drivers/i3c/master.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 0ea7bb045fad..bda4b9613e53 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1823,10 +1823,8 @@ int i3c_master_add_i3c_dev_locked(struct 
> i3c_master_controller *master,
>   return PTR_ERR(newdev);
>  
>   ret = i3c_master_attach_i3c_dev(master, newdev);
> - if (ret) {
> - ret = PTR_ERR(newdev);
> + if (ret)
>   goto err_free_dev;
> - }
>  
>   ret = i3c_master_retrieve_dev_info(newdev);
>   if (ret)



[PATCH] i3c: Rename dw-i3c-master.c into i3c-master-dw.c

2018-11-26 Thread Boris Brezillon
Follow the naming scheme introduced by the Cadence driver to keep things
consistent.

Signed-off-by: Boris Brezillon 
---
 drivers/i3c/master/Makefile | 2 +-
 drivers/i3c/master/{dw-i3c-master.c => i3c-master-dw.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/i3c/master/{dw-i3c-master.c => i3c-master-dw.c} (100%)

diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
index fc53939a0bb1..c5f52733aa10 100644
--- a/drivers/i3c/master/Makefile
+++ b/drivers/i3c/master/Makefile
@@ -1,2 +1,2 @@
 obj-$(CONFIG_CDNS_I3C_MASTER)  += i3c-master-cdns.o
-obj-$(CONFIG_DW_I3C_MASTER)+= dw-i3c-master.o
+obj-$(CONFIG_DW_I3C_MASTER)+= i3c-master-dw.o
diff --git a/drivers/i3c/master/dw-i3c-master.c 
b/drivers/i3c/master/i3c-master-dw.c
similarity index 100%
rename from drivers/i3c/master/dw-i3c-master.c
rename to drivers/i3c/master/i3c-master-dw.c
-- 
2.17.1



Re: [PATCH] mtd: spi-nor: fix erase_type array to indicate current map conf

2018-11-23 Thread Boris Brezillon
On Fri, 23 Nov 2018 11:17:29 +0100
Boris Brezillon  wrote:

> On Fri, 23 Nov 2018 09:42:55 +
> "Sverdlin, Alexander (Nokia - DE/Ulm)" 
> wrote:
> 
> > Hello Tudor,
> > 
> > On 22/11/2018 13:36, tudor.amba...@microchip.com wrote:
> > > From: Tudor Ambarus 
> > > 
> > > Bug reported for the out-of-tree S25FS128S flash memory.
> > > 
> > > BFPT table advertises all the erase types supported by all the
> > > possible map configurations. Update the erase_type array to indicate
> > > which erase types are applicable to the current map configuration.
> > > 
> > > Backward compatibility test done on sst26vf064b.
> > > 
> > > Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter 
> > > Table")
> > > Reported-by: Alexander Sverdlin 
> > > Signed-off-by: Tudor Ambarus   
> > 
> > I've tested this patch and it fixes the erasesize for S25FS128S and
> > our 128k partitions are writeable again with this patch.
> > 
> > Nevertheless, I think this is coincidence. I don't think that it
> > makes sense to OR all the erase types from all regions in one
> > bitmask and derive any uniform erasesize out of it.
> > This makes little sense for me in case of non-uniform maps.
> > 
> > I believe, the culprit here is one level higher, in the MTD partitioning
> > code (mtdpart.c) which has to be taught about non-uniform maps
> > but there is no infrastructure for this currently.
> 
> Keep in mind that mtdpart is only one part of the issue. As I said
> previously, some MTD users (UBI) expect a single eraseblock size, so
> it's not only mtdpart that you'd need to fix, but basically all MTD
> users that don't check ->eraseregions.

Just checked, and it seems mtdpart already supports eraseregions [1].
It picks the biggest erasesize of the regions covered by the partition,
which is exactly what we want. So all we'd have to do is make spi-nor.c
define those regions.

[1]https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/mtd/mtdpart.c#L482


Re: [PATCH] mtd: spi-nor: fix erase_type array to indicate current map conf

2018-11-23 Thread Boris Brezillon
On Fri, 23 Nov 2018 09:42:55 +
"Sverdlin, Alexander (Nokia - DE/Ulm)" 
wrote:

> Hello Tudor,
> 
> On 22/11/2018 13:36, tudor.amba...@microchip.com wrote:
> > From: Tudor Ambarus 
> > 
> > Bug reported for the out-of-tree S25FS128S flash memory.
> > 
> > BFPT table advertises all the erase types supported by all the
> > possible map configurations. Update the erase_type array to indicate
> > which erase types are applicable to the current map configuration.
> > 
> > Backward compatibility test done on sst26vf064b.
> > 
> > Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> > Reported-by: Alexander Sverdlin 
> > Signed-off-by: Tudor Ambarus   
> 
> I've tested this patch and it fixes the erasesize for S25FS128S and
> our 128k partitions are writeable again with this patch.
> 
> Nevertheless, I think this is coincidence. I don't think that it
> makes sense to OR all the erase types from all regions in one
> bitmask and derive any uniform erasesize out of it.
> This makes little sense for me in case of non-uniform maps.
> 
> I believe, the culprit here is one level higher, in the MTD partitioning
> code (mtdpart.c) which has to be taught about non-uniform maps
> but there is no infrastructure for this currently.

Keep in mind that mtdpart is only one part of the issue. As I said
previously, some MTD users (UBI) expect a single eraseblock size, so
it's not only mtdpart that you'd need to fix, but basically all MTD
users that don't check ->eraseregions.

> 
> What is possible to fix still is to choose smallest, not biggest
> erasesize for uniform case.

Yep, I agree. But we also have to be careful here, if some NORs were
already supported and were picking the biggest erasesize, we have to
preserve this behavior, otherwise we'll break things.

> I have such a patch, but I need hands
> on on a S25FS128S configured in uniform mode.
> 
> Non uniform case requires MTD layer rework.

There's already the concept of ->eraseregions. Maybe I'm wrong but I
thought it would fit the non-uniform erase scheme we have in SPI NOR. 

> We just cannot handle
> this hardware with just one erasesize in mind.

We can, if we decide to always use the biggest non-uniform erasesize.
Of course that only works if the biggest erase size is always a
multiple of smaller ones, but I'm pretty sure this is always true.

> 
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 29 -
> >  1 file changed, 28 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > index 93c9bc8931fc..a71adcd6ddfa 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -3000,7 +3000,8 @@ static int spi_nor_init_non_uniform_erase_map(struct 
> > spi_nor *nor,
> > u64 offset;
> > u32 region_count;
> > int i, j;
> > -   u8 erase_type, uniform_erase_type;
> > +   u8 uniform_erase_type, save_uniform_erase_type;
> > +   u8 erase_type, regions_erase_type;
> >  
> > region_count = SMPT_MAP_REGION_COUNT(*smpt);
> > /*
> > @@ -3014,6 +3015,7 @@ static int spi_nor_init_non_uniform_erase_map(struct 
> > spi_nor *nor,
> > map->regions = region;
> >  
> > uniform_erase_type = 0xff;
> > +   regions_erase_type = 0;
> > offset = 0;
> > /* Populate regions. */
> > for (i = 0; i < region_count; i++) {
> > @@ -3030,13 +3032,38 @@ static int 
> > spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
> >  */
> > uniform_erase_type &= erase_type;
> >  
> > +   /*
> > +* regions_erase_type mask will indicate all the erase types
> > +* supported in this configuration map.
> > +*/
> > +   regions_erase_type |= erase_type;
> > +
> > offset = (region[i].offset & ~SNOR_ERASE_FLAGS_MASK) +
> >  region[i].size;
> > }
> >  
> > +   save_uniform_erase_type = map->uniform_erase_type;
> > map->uniform_erase_type = spi_nor_sort_erase_mask(map,
> >   uniform_erase_type);
> >  
> > +   if (!regions_erase_type) {
> > +   /*
> > +* Roll back to the previous uniform_erase_type mask, SMPT is
> > +* broken.
> > +*/
> > +   map->uniform_erase_type = save_uniform_erase_type;
> > +   return -EINVAL;
> > +   }
> > +
> > +   /*
> > +* BFPT table advertises all the erase types supported by all the
> > +* possible map configurations. Update the erase_type array to indicate
> > +* which erase types are applicable to the current map configuration.
> > +*/
> > +   for (i = 0; i < SNOR_ERASE_TYPE_MAX; i++)
> > +   if (!(regions_erase_type & BIT(erase[i].idx)))
> > +   spi_nor_set_erase_type([i], 0, 0xFF);
> > +
> > spi_nor_region_mark_end([i - 1]);
> >  
> > return 0;  
> 



[GIT PULL] mtd: Fixes for 4.20-rc4

2018-11-22 Thread Boris Brezillon
Hello Linus,

Here is the MTD fixes PR for 4.20-rc4.

Regards,

Boris

The following changes since commit ccda4af0f4b92f7b4c308d3acc262f4a7e3affad:

  Linux 4.20-rc2 (2018-11-11 17:12:31 -0600)

are available in the Git repository at:

  git://git.infradead.org/linux-mtd.git tags/mtd/fixes-for-4.20-rc4

for you to fetch changes up to e8828ec1c003727fc001eab06aa19bd2ca9b677e:

  mtd: spi-nor: fix selection of uniform erase type in flexible conf 
(2018-11-20 14:26:59 +0100)


SPI NOR fixes:
- Various fixes related to the SFDP parsing code merged in 4.20
- Fix for a page fault in the cadence-qspi

NAND fixes:
- Fix a macro name conflict between the QCOM NAND controller driver
  and the RISC-V asm headers
- Fix of-node handling in the atmel driver


Johan Hovold (1):
  mtd: rawnand: atmel: fix OF child-node lookup

Olof Johansson (1):
  mtd: rawnand: qcom: Namespace prefix some commands

Thor Thayer (1):
  mtd: spi-nor: Fix Cadence QSPI page fault kernel panic

tudor.amba...@microchip.com (5):
  mtd: spi-nor: don't drop sfdp data if optional parsers fail
  mtd: spi-nor: fix iteration over smpt array
  mtd: spi-nor: don't overwrite errno in spi_nor_get_map_in_use()
  mtd: spi_nor: pass DMA-able buffer to spi_nor_read_raw()
  mtd: spi-nor: fix selection of uniform erase type in flexible conf

 drivers/mtd/nand/raw/atmel/nand-controller.c |  11 +++--
 drivers/mtd/nand/raw/qcom_nandc.c|  32 ++---
 drivers/mtd/spi-nor/cadence-quadspi.c|  19 ++--
 drivers/mtd/spi-nor/spi-nor.c| 130 
+++-
 4 files changed, 137 insertions(+), 55 deletions(-)


Re: Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O

2018-11-21 Thread Boris Brezillon
On Wed, 21 Nov 2018 12:08:02 +0100
Janusz Krzysztofik  wrote:

> Finalize implementation of the idea suggested by Artem Bityutskiy and
> Tony Lindgren, described in commit b027274d2e3a ("mtd: ams-delta: fix
> request_mem_region() failure"). Use pure GPIO consumer API, as reqested
> by Boris Brezillon.
> 
> Janusz Krzysztofik (4):
>   ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
>   mtd: rawnand: ams-delta: Request data port GPIO resource
>   mtd: rawnand: ams-delta: Use GPIO API for data I/O
>   ARM: OMAP1: ams-delta: Drop obsolete NAND resources
> 
>  arch/arm/mach-omap1/board-ams-delta.c |   22 ++
>  drivers/mtd/nand/raw/ams-delta.c  |  120 
> +++---
>  2 files changed, 80 insertions(+), 62 deletions(-)
> 
> Performance on Amstrad Delta is now acceptable after recent extensions
> to GPIO API and rawnanad enhancements.
> 
> Series intented to be merged via mtd tree, should not conflict with
> other OMAP1 patches submitted for 4.21.

We'll prepare an immutable tag, just in case.


Re: [PATCH v4 3/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O

2018-11-21 Thread Boris Brezillon
On Wed, 21 Nov 2018 12:08:05 +0100
Janusz Krzysztofik  wrote:

> Don't readw()/writew() data directly from/to GPIO port which is under
> control of gpio-omap driver, use GPIO consumer API instead.
> 
> The driver should now work with any 8-bit bidirectional GPIO port, not
> only OMAP.
> 
> Signed-off-by: Janusz Krzysztofik 
> Reviewed-by: Linus Walleij 

Reviewed-by: Boris Brezillon 

And thanks a lot for keeping up with that. I like the new ams-delta
driver, and I wonder if we couldn't extend it to replace the gpio-nand
driver.

> ---
>  drivers/mtd/nand/raw/ams-delta.c | 109 +--
>  1 file changed, 61 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/ams-delta.c 
> b/drivers/mtd/nand/raw/ams-delta.c
> index bb50dda05654..8312182088c1 100644
> --- a/drivers/mtd/nand/raw/ams-delta.c
> +++ b/drivers/mtd/nand/raw/ams-delta.c
> @@ -18,11 +18,10 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  
>  /*
> @@ -38,7 +37,7 @@ struct ams_delta_nand {
>   struct gpio_desc*gpiod_nwe;
>   struct gpio_desc*gpiod_ale;
>   struct gpio_desc*gpiod_cle;
> - void __iomem*io_base;
> + struct gpio_descs   *data_gpiods;
>   booldata_in;
>  };
>  
> @@ -67,42 +66,78 @@ static const struct mtd_partition partition_info[] = {
> .size =  3 * SZ_256K },
>  };
>  
> -static void ams_delta_io_write(struct ams_delta_nand *priv, u8 byte)
> +static void ams_delta_write_commit(struct ams_delta_nand *priv)
>  {
> - writew(byte, priv->io_base + OMAP_MPUIO_OUTPUT);
>   gpiod_set_value(priv->gpiod_nwe, 0);
>   ndelay(40);
>   gpiod_set_value(priv->gpiod_nwe, 1);
>  }
>  
> +static void ams_delta_io_write(struct ams_delta_nand *priv, u8 byte)
> +{
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + DECLARE_BITMAP(values, BITS_PER_TYPE(byte)) = { byte, };
> +
> + gpiod_set_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
> +   data_gpiods->info, values);
> +
> + ams_delta_write_commit(priv);
> +}
> +
> +static void ams_delta_dir_output(struct ams_delta_nand *priv, u8 byte)
> +{
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + DECLARE_BITMAP(values, BITS_PER_TYPE(byte)) = { byte, };
> + int i;
> +
> + for (i = 0; i < data_gpiods->ndescs; i++)
> + gpiod_direction_output_raw(data_gpiods->desc[i],
> +test_bit(i, values));
> +
> + ams_delta_write_commit(priv);
> +
> + priv->data_in = false;
> +}
> +
>  static u8 ams_delta_io_read(struct ams_delta_nand *priv)
>  {
>   u8 res;
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + DECLARE_BITMAP(values, BITS_PER_TYPE(res)) = { 0, };
>  
>   gpiod_set_value(priv->gpiod_nre, 0);
>   ndelay(40);
> - res = readw(priv->io_base + OMAP_MPUIO_INPUT_LATCH);
> +
> + gpiod_get_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
> +   data_gpiods->info, values);
> +
>   gpiod_set_value(priv->gpiod_nre, 1);
>  
> + res = values[0];
>   return res;
>  }
>  
> -static void ams_delta_dir_input(struct ams_delta_nand *priv, bool in)
> +static void ams_delta_dir_input(struct ams_delta_nand *priv)
>  {
> - writew(in ? ~0 : 0, priv->io_base + OMAP_MPUIO_IO_CNTL);
> - priv->data_in = in;
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + int i;
> +
> + for (i = 0; i < data_gpiods->ndescs; i++)
> + gpiod_direction_input(data_gpiods->desc[i]);
> +
> + priv->data_in = true;
>  }
>  
>  static void ams_delta_write_buf(struct ams_delta_nand *priv, const u8 *buf,
>   int len)
>  {
> - int i;
> + int i = 0;
>  
> - if (priv->data_in)
> - ams_delta_dir_input(priv, false);
> + if (len > 0 && priv->data_in)
> + ams_delta_dir_output(priv, buf[i++]);
>  
> - for (i = 0; i < len; i++)
> - ams_delta_io_write(priv, buf[i]);
> + while (i < len)
> + ams_delta_io_write(priv, buf[i++]);
>  }
>  
>  static void ams_delta_read_buf(struct ams_delta_nand *priv, u8 *buf, int len)
> @@ -110,7 +145,7 @@ static void ams_delta_read_buf(struct ams_delta_nand 
> *priv, u8 *buf, int len)
>   int i;
>  
>   if (!priv->data_in)
> -   

Re: [v3,1/2] mtd: spi-nor: add macros related to MICRON flash

2018-11-21 Thread Boris Brezillon
On Fri, 2018-10-12 at 02:23:08 UTC, Yogesh Narayan Gaur wrote:
> Some MICRON related macros in spi-nor domain were ST.
> Rename entries related to STMicroelectronics under macro SNOR_MFR_ST.
> 
> Added entry of MFR Id for Micron flashes, 0x002C.
> 
> Signed-off-by: Yogesh Gaur 
> Reviewed-by: Tudor Ambarus 

Applied to http://git.infradead.org/linux-mtd.git spi-nor/next, thanks.

Boris


Re: [v3,2/2] mtd: spi-nor: add entry for mt35xu512aba flash

2018-11-21 Thread Boris Brezillon
On Fri, 2018-10-12 at 02:23:13 UTC, Yogesh Narayan Gaur wrote:
> Add entry for mt35xu512aba Micron NOR flash.
> This flash is having uniform sector erase size of 128KB, have
> support of FSR(flag status register), flash size is 64MB and
> supports 4-byte commands.
> 
> Signed-off-by: Yogesh Gaur 
> Reviewed-by: Tudor Ambarus 

Applied to http://git.infradead.org/linux-mtd.git spi-nor/next, thanks.

Boris


Re: [v2] mtd: spi-nor: fix selection of uniform erase type in flexible conf

2018-11-21 Thread Boris Brezillon
On Fri, 2018-11-16 at 17:46:37 UTC,  wrote:
> There are uniform, non-uniform and flexible erase flash configurations.
> 
> The non-uniform erase types, are the erase types that can _not_ erase
> the entire flash by their own.
> 
> As the code was, in case flashes had flexible erase capabilities
> (support both uniform and non-uniform erase types in the same flash
> configuration) and supported multiple uniform erase type sizes, the
> code did not sort the uniform erase types, and could select a wrong
> erase type size.
> 
> Sort the uniform erase mask in case of flexible erase flash
> configurations, in order to select the best uniform erase type size.
> 
> Uniform, non-uniform, and flexible configurations with just a valid
> uniform erase type, are not affected by this change.
> 
> Uniform erase tested on mx25l3273fm2i-08g and sst26vf064B-104i/sn.
> Non uniform erase tested on sst26vf064B-104i/sn.
> 
> Fixes: 5390a8df769e ("mtd: spi-nor: add support to non-uniform SFDP SPI NOR 
> flash memories")
> Signed-off-by: Tudor Ambarus 

Applied to http://git.infradead.org/linux-mtd.git master, thanks.

Boris


Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller

2018-11-20 Thread Boris Brezillon
On Fri, 9 Nov 2018 10:30:41 +0530
Naga Sureshkumar Relli  wrote:

> +static int anfc_setup_data_interface(struct nand_chip *chip, int csline,
> +  const struct nand_data_interface *conf)
> +{
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + int err;
> + const struct nand_sdr_timings *sdr;
> + u32 inftimeval;
> + bool change_sdr_clk = false;
> +
> + if (csline == NAND_DATA_IFACE_CHECK_ONLY)
> + return 0;
> +
> + /*
> +  * If the controller is already in the same mode as flash device
> +  * then no need to change the timing mode again.
> +  */
> + sdr = nand_get_sdr_timings(conf);
> + if (IS_ERR(sdr))
> + return PTR_ERR(sdr);
> +
> + if (sdr->mode < 0)
> + return -ENOTSUPP;
> +
> + inftimeval = sdr->mode & 7;
> + if (sdr->mode >= 2 && sdr->mode <= 5)
> + change_sdr_clk = true;
> + /*
> +  * SDR timing modes 2-5 will not work for the arasan nand when
> +  * freq > 90 MHz, so reduce the freq in SDR modes 2-5 to < 90Mhz

What's the freq for mode 0 and 1?

> +  */
> + if (change_sdr_clk) {
> + clk_disable_unprepare(nfc->clk_sys);
> + err = clk_set_rate(nfc->clk_sys, SDR_MODE_DEFLT_FREQ);

You should not change the clk rate here. It should be done when the
chip is selected, so that, if you ever have 2 different chips connected
to the same controller, you can adapt the rate when they are accessed.

> + if (err) {
> + dev_err(nfc->dev, "Can't set the clock rate\n");
> + return err;
> + }
> + err = clk_prepare_enable(nfc->clk_sys);
> + if (err) {
> + dev_err(nfc->dev, "Unable to enable sys clock.\n");
> + clk_disable_unprepare(nfc->clk_sys);
> + return err;
> + }
> + }
> + achip->inftimeval = inftimeval;
> +
> + return 0;
> +}
> +
> +static int anfc_nand_attach_chip(struct nand_chip *chip)
> +{
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + u32 ret;
> +
> + if (mtd->writesize <= SZ_512)
> + achip->spare_caddr_cycles = 1;
> + else
> + achip->spare_caddr_cycles = 2;
> +
> + chip->ecc.calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> + chip->ecc.code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);

Those bufs are allocated but never freed (memleak). Also, are you sure
you really need them.


Re: [BUG BISECT] Missing MTD NAND partitions - mtd: rawnand: Move the ->exec_op() method to nand_controller_ops

2018-11-20 Thread Boris Brezillon
On Tue, 20 Nov 2018 12:48:46 +0100
Krzysztof Kozlowski  wrote:

> On Tue, 20 Nov 2018 at 11:33, Miquel Raynal  wrote:
> >
> > Hi Krzysztof,
> >
> > Krzysztof Kozłowski  wrote on Tue, 20 Nov 2018
> > 10:46:33 +0100:
> >  
> > > Hi all,
> > >
> > > Since few days linux-next has problem on Freescale VF500 - MTD seems
> > > to be broken.
> > >
> > > Bisect pointed me to commit 7c27338c728e39ef47c83d101959aa332506969d
> > > ("mtd: rawnand: Move the ->exec_op() method to nand_controller_ops")
> > > as reason of failure to find MTD partitions.
> > >
> > > Toradex Colibri VF50 on Iris board (ARMv7, UP, Cortext-A5, NXP VF500,
> > > 128 MB RAM, 128 MB NAND, Systemd: v232) booted from NFS root (NFSv4)
> > > trying to mount UBIFS from NAND/MTD. Board uses VF610 NAND driver.
> > >
> > > The MTD partitions are missing entirely (nothing under /dev/mtd).
> > > In the logs you can also see:
> > > [1.232161] UBI error: cannot open mtd ubi2, error -2
> > >
> > > Attached - dmesg.log
> > >
> > > Let me know if you need defconfig or any other information.  
> >
> > Thank you very much for testing and reporting the bug!
> >
> > Could you please test with this diff applied [1] please? We discussed
> > with Boris and we think it should fix your setup (and all others also
> > impacted).
> >
> > [1] http://code.bulix.org/r1m99i-509201  
> 
> error: patch failed: drivers/mtd/nand/raw/nand_base.c:4399
> error: drivers/mtd/nand/raw/nand_base.c: patch does not apply
> 
> Maybe you have a git tree with this somewhere?

Yep: https://github.com/bbrezillon/linux/tree/nand/next-fix


Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller

2018-11-20 Thread Boris Brezillon
On Tue, 20 Nov 2018 07:02:08 +
Naga Sureshkumar Relli  wrote:


> > 
> > Can you please run nandbiterrs (availaible in mtd-utils). I fear your
> > device won't pass the test.  
> Yes, nandbiterror test is passing till 24bit, after that it is failing.

Can you paste the output of nandbiterrs please?

> >   
> > > But we are hitting this because of erased page reading(needed in case of 
> > > ubifs).
> > >  
> > > >
> > > > Don't you have a bit (or several bits) reporting when the ECC engine 
> > > > was not able to  
> > correct  
> > > > data? I you do, you should base the "detect bitflips in erase pages" 
> > > > logic on this information.  
> > > Bit reporting for several bit errors is there only for Hamming(1bit 
> > > correction and 2bit  
> > detection) but not in BCH.  
> > >  
> > 
> > Then I tend to agree with Miquel: your ECC engine is broken, and I'm
> > not even sure how to deal with that yet.  
> So as per the Miquel's suggestion, can I proceed to add the below one?
> "you should re-read the page in raw mode and check for the number of bitflips 
> manually (thanks to the helpers in the core). Again, if the number of BF is 
> above 16, we can assume the page is bad and increment ->ecc.failed 
> accordingly."

But that's just partially fixing the problem. And you didn't answer my
previous question: what happens when you configure the ECC engine in,
say 12bit/1024 and you end up with uncorrectable errors (more than 12
bitflips in a 1k block). What's the number reported ECC_ERR_CNT? Is it
set to 13?


Re: [PATCHv2] mtd: spi-nor: Fix Cadence QSPI page fault kernel panic

2018-11-20 Thread Boris Brezillon
On Fri, 2018-11-16 at 14:25:49 UTC, thor.tha...@linux.intel.com wrote:
> From: Thor Thayer 
> 
> The current Cadence QSPI driver caused a kernel panic sporadically
> when writing to QSPI. The problem was caused by writing more bytes
> than needed because the QSPI operated on 4 bytes at a time.
> 
> [   11.202044] Unable to handle kernel paging request at virtual address 
> bffd3000
> [   11.209254] pgd = e463054d
> [   11.211948] [bffd3000] *pgd=2fffb811, *pte=, *ppte=
> [   11.218202] Internal error: Oops: 7 [#1] SMP ARM
> [   11.222797] Modules linked in:
> [   11.225844] CPU: 1 PID: 1317 Comm: systemd-hwdb Not tainted 
> 4.17.7-d0c45cd44a8f
> [   11.235796] Hardware name: Altera SOCFPGA Arria10
> [   11.240487] PC is at __raw_writesl+0x70/0xd4
> [   11.244741] LR is at cqspi_write+0x1a0/0x2cc
> 
> On a page boundary limit the number of bytes copied from the tx buffer
> to remain within the page.
> 
> This patch uses a temporary buffer to hold the 4 bytes to write and then
> copies only the bytes required from the tx buffer.
> 
> Reported-by: Adrian Amborzewicz 
> Signed-off-by: Thor Thayer 

Applied to http://git.infradead.org/linux-mtd.git master, thanks.

Boris


Re: [PATCH][i3c-next] i3c: master: fix mask operation by using the correct operator

2018-11-19 Thread Boris Brezillon
On Mon, 19 Nov 2018 12:41:10 +
vitor  wrote:

> Hi Coling,
> 
> 
> Thanks for your report.
> 
> On 16/11/18 18:42, Colin King wrote:
> > From: Colin Ian King 
> >
> > The masking operation on status is using a bitwise 'or' rather than
> > a bitwise 'and' operator, and hence the result is always non-zero
> > which is probably not what is intended. Fix this by using the correct
> > operator.
> >
> > Detected by CoverityScan, CID#1475523 ("Wrong operator used")
> >
> > Fixes: 88acc98a712a ("i3c: master: Add driver for Synopsys DesignWare IP")
> > Signed-off-by: Colin Ian King 
> > ---
> >   drivers/i3c/master/dw-i3c-master.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/i3c/master/dw-i3c-master.c 
> > b/drivers/i3c/master/dw-i3c-master.c
> > index 0153e6e9de52..b532e2c9cf5c 100644
> > --- a/drivers/i3c/master/dw-i3c-master.c
> > +++ b/drivers/i3c/master/dw-i3c-master.c
> > @@ -1085,7 +1085,7 @@ static irqreturn_t dw_i3c_master_irq_handler(int irq, 
> > void *dev_id)
> >   
> > spin_lock(>xferqueue.lock);
> > dw_i3c_master_end_xfer_locked(master, status);
> > -   if (status | INTR_TRANSFER_ERR_STAT)
> > +   if (status & INTR_TRANSFER_ERR_STAT)
> > writel(INTR_TRANSFER_ERR_STAT, master->regs + INTR_STATUS);
> > spin_unlock(>xferqueue.lock);
> > 
> Acked-by: Vitor Soares 

Applied.

Thanks,

Boris


Re: [PATCH 2/2] dt-binding: spi: Document Renesas R-Car RPC controller bindings

2018-11-19 Thread Boris Brezillon
On Mon, 19 Nov 2018 16:12:41 +0100
Marek Vasut  wrote:

> On 11/19/2018 03:43 PM, Boris Brezillon wrote:
> > On Mon, 19 Nov 2018 15:14:07 +0100
> > Marek Vasut  wrote:
> >   
> >> On 11/19/2018 03:10 PM, Boris Brezillon wrote:  
> >>> On Mon, 19 Nov 2018 14:49:31 +0100
> >>> Marek Vasut  wrote:
> >>> 
> >>>> On 11/19/2018 11:01 AM, Mason Yang wrote:
> >>>>> Document the bindings used by the Renesas R-Car D3 RPC controller.
> >>>>>
> >>>>> Signed-off-by: Mason Yang 
> >>>>> ---
> >>>>>  .../devicetree/bindings/spi/spi-renesas-rpc.txt| 33 
> >>>>> ++
> >>>>>  1 file changed, 33 insertions(+)
> >>>>>  create mode 100644 
> >>>>> Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt 
> >>>>> b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>> new file mode 100644
> >>>>> index 000..8286cc8
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>> @@ -0,0 +1,33 @@
> >>>>> +Renesas R-Car D3 RPC controller Device Tree Bindings
> >>>>> +
> >>>>> +
> >>>>> +Required properties:
> >>>>> +- compatible: should be "renesas,rpc-r8a77995"
> >>>>> +- #address-cells: should be 1
> >>>>> +- #size-cells: should be 0
> >>>>> +- reg: should contain 2 entries, one for the registers and one for the 
> >>>>> direct
> >>>>> +   mapping area
> >>>>> +- reg-names: should contain "rpc_regs" and "dirmap"
> >>>>> +- interrupts: interrupt line connected to the RPC SPI controller  
> >>>>
> >>>> Do you also plan to support the RPC HF mode ? And if so, how would that
> >>>> look in the bindings ?
> >>>
> >>> Not sure this approach is still accepted, but that's how we solved the
> >>> problem for the flexcom block [1].
> >>>
> >>> [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> >>> 
> >>
> >> That looks pretty horrible.
> >>
> >> In U-Boot we check whether the device hanging under the controller node
> >> is JEDEC SPI flash or CFI flash and based on that decide what the config
> >> of the controller should be (SPI or HF). Not sure that's much better,but
> >> at least it doesn't need extra nodes which do not really represent any
> >> kind of real hardware.
> >>  
> > 
> > The subnodes are not needed, you can just have a property that tells in
> > which mode the controller is supposed to operate, and the MFD would
> > create a sub-device that points to the same device_node.  
> 
> Do you even need a dedicated property ? I think you can decide purely on
> what node is hanging under the controller (jedec spi nor or cfi nor).

Yes, that could work if they have well-known compatibles. As soon as
people start using flash-specific compats (like some people do for
their SPI NORs) it becomes a maintenance burden.

> 
> > Or we can have
> > a single driver that decides what to declare (a spi_controller or flash
> > controller), but you'd still have to decide where to place this
> > driver...  
> 
> I'd definitely prefer a single driver.
> 

Where would you put this driver? I really don't like the idea of having
MTD drivers spread over the tree. Don't know what's Mark's opinion on
this matter.


Re: [PATCH 2/2] dt-binding: spi: Document Renesas R-Car RPC controller bindings

2018-11-19 Thread Boris Brezillon
On Mon, 19 Nov 2018 15:14:07 +0100
Marek Vasut  wrote:

> On 11/19/2018 03:10 PM, Boris Brezillon wrote:
> > On Mon, 19 Nov 2018 14:49:31 +0100
> > Marek Vasut  wrote:
> >   
> >> On 11/19/2018 11:01 AM, Mason Yang wrote:  
> >>> Document the bindings used by the Renesas R-Car D3 RPC controller.
> >>>
> >>> Signed-off-by: Mason Yang 
> >>> ---
> >>>  .../devicetree/bindings/spi/spi-renesas-rpc.txt| 33 
> >>> ++
> >>>  1 file changed, 33 insertions(+)
> >>>  create mode 100644 
> >>> Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt 
> >>> b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>> new file mode 100644
> >>> index 000..8286cc8
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>> @@ -0,0 +1,33 @@
> >>> +Renesas R-Car D3 RPC controller Device Tree Bindings
> >>> +
> >>> +
> >>> +Required properties:
> >>> +- compatible: should be "renesas,rpc-r8a77995"
> >>> +- #address-cells: should be 1
> >>> +- #size-cells: should be 0
> >>> +- reg: should contain 2 entries, one for the registers and one for the 
> >>> direct
> >>> +   mapping area
> >>> +- reg-names: should contain "rpc_regs" and "dirmap"
> >>> +- interrupts: interrupt line connected to the RPC SPI controller
> >>
> >> Do you also plan to support the RPC HF mode ? And if so, how would that
> >> look in the bindings ?  
> > 
> > Not sure this approach is still accepted, but that's how we solved the
> > problem for the flexcom block [1].
> > 
> > [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> >   
> 
> That looks pretty horrible.
> 
> In U-Boot we check whether the device hanging under the controller node
> is JEDEC SPI flash or CFI flash and based on that decide what the config
> of the controller should be (SPI or HF). Not sure that's much better,but
> at least it doesn't need extra nodes which do not really represent any
> kind of real hardware.
> 

The subnodes are not needed, you can just have a property that tells in
which mode the controller is supposed to operate, and the MFD would
create a sub-device that points to the same device_node. Or we can have
a single driver that decides what to declare (a spi_controller or flash
controller), but you'd still have to decide where to place this
driver...


Re: [PATCH 2/2] dt-binding: spi: Document Renesas R-Car RPC controller bindings

2018-11-19 Thread Boris Brezillon
On Mon, 19 Nov 2018 14:49:31 +0100
Marek Vasut  wrote:

> On 11/19/2018 11:01 AM, Mason Yang wrote:
> > Document the bindings used by the Renesas R-Car D3 RPC controller.
> > 
> > Signed-off-by: Mason Yang 
> > ---
> >  .../devicetree/bindings/spi/spi-renesas-rpc.txt| 33 
> > ++
> >  1 file changed, 33 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt 
> > b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> > new file mode 100644
> > index 000..8286cc8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> > @@ -0,0 +1,33 @@
> > +Renesas R-Car D3 RPC controller Device Tree Bindings
> > +
> > +
> > +Required properties:
> > +- compatible: should be "renesas,rpc-r8a77995"
> > +- #address-cells: should be 1
> > +- #size-cells: should be 0
> > +- reg: should contain 2 entries, one for the registers and one for the 
> > direct
> > +   mapping area
> > +- reg-names: should contain "rpc_regs" and "dirmap"
> > +- interrupts: interrupt line connected to the RPC SPI controller  
> 
> Do you also plan to support the RPC HF mode ? And if so, how would that
> look in the bindings ?

Not sure this approach is still accepted, but that's how we solved the
problem for the flexcom block [1].

[1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt


Re: [PATCH v10 0/9] Add the I3C subsystem

2018-11-19 Thread Boris Brezillon
On Mon, 19 Nov 2018 12:35:42 +
vitor  wrote:

> Hi Boris,
> 
> On 16/11/18 13:16, Boris Brezillon wrote:
> > On Fri, 16 Nov 2018 12:31:42 +
> > vitor  wrote:
> >  
> >> Hi Boris,
> >>
> >>
> >> On 15/11/18 19:00, Boris Brezillon wrote:  
> >>> On Thu, 15 Nov 2018 18:03:47 +
> >>> vitor  wrote:
> >>> 
> >>>> Hi Boris,
> >>>>
> >>>>
> >>>> On 15/11/18 15:28, Boris Brezillon wrote:  
> >>>>> On Thu, 15 Nov 2018 16:01:37 +0100
> >>>>> Wolfram Sang  wrote:
> >>>>>
> >>>>>> Hi Boris,
> >>>>>>
> >>>>>>> What we could do though, is expose I3C devices that do not have a
> >>>>>>> driver in kernel space, like spidev does.  
> >>>>>> ...
> >>>>>>
> >>>>>>> Mark, Wolfram, Arnd, Greg, any opinion?  
> >>>>>> Is there a benefit for having drivers in userspace? My gut feeling is 
> >>>>>> to
> >>>>>> encourage people to write kernel drivers. If this is, for some reason,
> >>>>>> not possible for some driver, then we have a use case at hand to test
> >>>>>> the then-to-be-developed userspace interface against. Until then, I
> >>>>>> personally wouldn't waste effort on designing it without a user in
> >>>>>> sight.  
> >>>>> I kind of agree with that. Vitor, do you have a use case in mind for
> >>>>> such userspace drivers? I don't think it's worth designing an API for
> >>>>> something we don't need (yet).  
> >>>> My use case is a tool for tests, lets say like the i2c tools.  
> >>> What would you like to test exactly?
> >>> 
> >>>> There is
> >>>> other subsystems, some of them mentioned on this thread, that have and
> >>>> ioctl system call or other method to change parameters or send data.  
> >>> I don't think they added the /dev interface before having a real use
> >>> case for it.
> >>> 
> >>>> I rise this topic because I really think it worth to define now how this
> >>>> should be design (and for me how to do the things right) to avoid future
> >>>> issues.  
> >>> Actually it should be done the other way around: you should have a real
> >>> need and the /dev interface should be designed to fulfill this need.
> >>> Based on this real use case we can discuss other potential usage that
> >>> might appear in the future and try to design something more
> >>> future-proof, but clearly, this userspace interface should be driven by
> >>> a real/well-defined use case.
> >>>
> >>> Also, exposing things to userspace is way more risky than adding a new
> >>> in-kernel subsystem/framework, because it then becomes part of the
> >>> stable ABI.
> >>>
> >>> To make things clearer, I'm not against the idea of exposing I3C
> >>> devices (or I3C buses) to userspace, but I'd like to understand what you
> >>> plan to do with that. If this is about testing, what kind of tests
> >>> you'd like to run. If this is about developing drivers in userspace,
> >>> why can't these be done in kernel space (license issues?), and what
> >>> would those drivers be allowed to do?  
> >>
> >> Basically I need a tool that help me during the development and to avoid
> >> me to write a dummy driver for each device that I test.  
> > But we want I3C device drivers to be upstreamed, so why not developing a
> > real driver everytime you test a new device and submitting it upstream?  
> 
> 
> Usually the devices that I test aren't the final product so it isn't 
> easy to do the upstream.
> 
> But when possible I plan to do that.
> 
> 
> >  
> >> For instances do some read/write,  
> > Doing SDR/DDR transfers is probably acceptable, but I still think we
> > should push hard to have kernel drivers when that's possible.
> >  
> >> get/set ccc commands,  
> > Exposing CCC commands is definitely not a good idea, since they're not
> > even exposed to kernel drivers.
> >  
> >> if something
> >> goes wrong during the bus initialization have a to debug etc...  
> > Can't we add such a debug infrastructure in the kernel. Maybe we can
> > expose debugfs files too if that helps, though if those debugfs files
> > are actually used by userspace libs/tools, it's not any better than
> > ioctls or sysfs files, since they will anyway become a stable ABI.
> >  
> >>
> >> For me this is a valid use case and I imagine when people start to
> >> develop in i3c this interface will help everyone.  
> > How about you propose an i3cdev driver that allow users to do SDR
> > transfers throuh an ioctl?  
> 
> I think that was for v6 I started to something to expose the bus like in 
> i2c-dev, but I liked the idea of expose only the device doesn't have a 
> driver. Do you know if  there is already something in the kernel doing 
> the same?

I know [1], but there might be other subsystems doing the same thing.

[1]https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/spi/spidev.c


Re: [PATCH v2 00/25] at24: remove

2018-11-19 Thread Boris Brezillon
On Mon, 19 Nov 2018 09:58:46 +0100
Bartosz Golaszewski  wrote:

> niedz., 18 lis 2018 o 17:03 Boris Brezillon
>  napisał(a):
> >
> > On Tue, 13 Nov 2018 15:01:08 +0100
> > Bartosz Golaszewski  wrote:
> >  
> > > As far as merging of this series goes: I'd like to avoid dragging it over
> > > four releases. The series is logically split into five groups:
> > >
> > > patches 1-2: nvmem and mtd changes
> > > patches 3-9: davinci arch-specific changes
> > > patches 10-13: networking changes
> > > patches 14-24: davinci specific again
> > > patch 25: final at24 change
> > >
> > > With that I believe we can do the following: Greg KH could pick up the
> > > first two patches into his char-misc tree.  
> >
> > The char-misc tree? Why not the MTD or NVMEM tree?
> >  
> 
> There is no NVMEM tree - Srinivas sends his patches to Greg to be
> applied to char-misc.

Okay, I didn't know that.

> 
> The second patch depends on the first one so in order to avoid more
> merging problems I suggested that the MTD patch go through char-misc
> as well. If you see a better solution, please let me know.

No that's fine, as long as I have an immutable tag/branch I can pull if
needed.


Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller

2018-11-19 Thread Boris Brezillon
On Mon, 19 Nov 2018 06:20:28 +
Naga Sureshkumar Relli  wrote:

> H Boris,
> 
> > -Original Message-
> > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> > Sent: Monday, November 19, 2018 1:13 AM
> > To: Naga Sureshkumar Relli 
> > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org;
> > computersforpe...@gmail.com; marek.va...@gmail.com; 
> > linux-...@lists.infradead.org; linux-
> > ker...@vger.kernel.org; nagasures...@gmail.com; r...@kernel.org; Michal 
> > Simek
> > 
> > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for 
> > Arasan NAND
> > Flash Controller
> > 
> > On Thu, 15 Nov 2018 09:34:16 +
> > Naga Sureshkumar Relli  wrote:
> >   
> > > Hi Boris & Miquel,
> > >
> > > I am updating the driver by addressing your comments, and I have one
> > > concern,  especially in anfc_read_page_hwecc(), there I am checking for 
> > > erased pages bit flips.
> > > Since Arasan NAND controller doesn't have multibit error detection
> > > beyond 24-bit( it can correct up to 24 bit), i.e. there is no indication 
> > > from controller to detect  
> > uncorrectable error beyond 24bit.
> > 
> > Do you mean that you can't detect uncorrectable errors, or just that it's 
> > not 100% sure to detect
> > errors above max_strength?  
> Yes, in Arasan NAND controller there is no way to detect uncorrectable errors 
> beyond 24-bit.

So how do you detect uncorrectable errors when the strength is less than
24bits?

> >   
> > > So I took some error count as default value(MULTI_BIT_ERR_CNT  16, I
> > > put this based on the error count that I got while reading erased page on 
> > > Micron device).
> > > And during a page read, will just read the error count register and
> > > compare this value with the default error count(16) and if it is more 
> > > Than default then I am  
> > checking for erased page bit flips.
> > 
> > Hm, that's wrong, especially if you set ecc_strength to something > 16.  
> Ok
> >   
> > > I am doubting that this will not work in all cases.  
> > 
> > It definitely doesn't.  
> Ok
> >   
> > > In my case it is just working because the error count that it got on an 
> > > erased page is 16.
> > > Could you please suggest a way to do detect erased_page bit flips when 
> > > reading a page with  
> > HW-ECC?.
> > 
> > I'm a bit lost. Is the problem only about bitflips in erase pages, or is it 
> > also impacting reads of
> > written pages that lead to uncorrectable errors.  
> Yes, it is for both. But in case of read errors that we can't detect beyond 
> 24-bit, then the answer from HW design team
> Is that the flash part is bad.
> Unfortunately till now we haven't ran into that situation(read errors of 
> written pages beyond 24-bit).

Can you please run nandbiterrs (availaible in mtd-utils). I fear your
device won't pass the test.

> But we are hitting this because of erased page reading(needed in case of 
> ubifs).
> 
> > 
> > Don't you have a bit (or several bits) reporting when the ECC engine was 
> > not able to correct
> > data? I you do, you should base the "detect bitflips in erase pages" logic 
> > on this information.  
> Bit reporting for several bit errors is there only for Hamming(1bit 
> correction and 2bit detection) but not in BCH.
> 

Then I tend to agree with Miquel: your ECC engine is broken, and I'm
not even sure how to deal with that yet.


Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller

2018-11-18 Thread Boris Brezillon
On Thu, 15 Nov 2018 09:34:16 +
Naga Sureshkumar Relli  wrote:

> Hi Boris & Miquel,
> 
> I am updating the driver by addressing your comments, and I have one concern, 
>  especially in anfc_read_page_hwecc(), 
> there I am checking for erased pages bit flips.
> Since Arasan NAND controller doesn't have multibit error detection beyond 
> 24-bit( it can correct up to 24 bit),
> i.e. there is no indication from controller to detect uncorrectable error 
> beyond 24bit.

Do you mean that you can't detect uncorrectable errors, or just that
it's not 100% sure to detect errors above max_strength?

> So I took some error count as default value(MULTI_BIT_ERR_CNT  16, I put this 
> based on the error count that 
> I got while reading erased page on Micron device).
> And during a page read, will just read the error count register and compare 
> this value with the default error count(16) and if it is more 
> Than default then I am checking for erased page bit flips.

Hm, that's wrong, especially if you set ecc_strength to something > 16.

> I am doubting that this will not work in all cases.

It definitely doesn't.

> In my case it is just working because the error count that it got on an 
> erased page is 16.
> Could you please suggest a way to do detect erased_page bit flips when 
> reading a page with HW-ECC?.

I'm a bit lost. Is the problem only about bitflips in erase pages, or
is it also impacting reads of written pages that lead to uncorrectable
errors.

Don't you have a bit (or several bits) reporting when the ECC engine
was not able to correct data? I you do, you should base the "detect
bitflips in erase pages" logic on this information.  


Re: [PATCH v2 5/9] mtd: nand: atmel: fix OF child-node lookup

2018-11-18 Thread Boris Brezillon
On Thu, 15 Nov 2018 15:26:48 +0100
Johan Hovold  wrote:

> On Tue, Oct 23, 2018 at 08:51:17PM +0200, Boris Brezillon wrote:
> > On Tue, 23 Oct 2018 13:28:09 -0500
> > Rob Herring  wrote:
> >   
> > > On Mon, Aug 27, 2018 at 4:44 AM Johan Hovold  wrote:  
> > > >
> > > > On Mon, Aug 27, 2018 at 10:48:42AM +0200, Boris Brezillon wrote:
> > > > > On Mon, 27 Aug 2018 10:44:14 +0200
> > > > > Johan Hovold  wrote:
> > > > >
> > > > > > On Mon, Aug 27, 2018 at 10:28:20AM +0200, Boris Brezillon wrote:
> > > > > > > Hi Johan
> > > > > > >
> > > > > > > On Mon, 27 Aug 2018 10:21:49 +0200
> > > > > > > Johan Hovold  wrote:
> > > > > > >
> > > > > > > > Use the new of_get_compatible_child() helper to lookup the nfc 
> > > > > > > > child
> > > > > > > > node instead of using of_find_compatible_node(), which searches 
> > > > > > > > the
> > > > > > > > entire tree from a given start node and thus can return an 
> > > > > > > > unrelated
> > > > > > > > (i.e. non-child) node.
> > > > > > > >
> > > > > > > > This also addresses a potential use-after-free (e.g. after probe
> > > > > > > > deferral) as the tree-wide helper drops a reference to its first
> > > > > > > > argument (i.e. the node of the device being probed).
> > > > > > > >
> > > > > > > > While at it, also fix a related nfc-node reference leak.
> > > > > > > >
> > > > > > > > Fixes: f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand 
> > > > > > > > driver")
> > > > > > > > Cc: stable  # 4.11
> > > > > > > > Cc: Nicolas Ferre 
> > > > > > > > Cc: Josh Wu 
> > > > > > > > Cc: Boris Brezillon 
> > > > > > > > Signed-off-by: Johan Hovold 
> > > > > > >
> > > > > > > Acked-by: Boris Brezillon 
> > > > > >
> > > > > > Thanks for the ack.
> > > > > >
> > > > > > > I'll let Miquel queue this patch to the nand/next branch, unless 
> > > > > > > you
> > > > > > > want it to be merged in 4.19, in which case I'll queue it to the
> > > > > > > mtd/fixes branch.
> > > > > >
> > > > > > Note that there's a dependency on the first patch of the series 
> > > > > > which
> > > > > > adds the new helper.
> > > > >
> > > > > I was not Cc-ed on this patch :P.
> > > >
> > > > Yeah, sorry about that. I made sure everyone was CCed on the
> > > > cover letter, but guess I could have reused that list for the helper as
> > > > well.
> > > >
> > > > > > Rob can pick up the entire series if the various
> > > > > > maintainers agree, otherwise I'll try to get at the least the helper
> > > > > > into -rc2.
> > > > >
> > > > > If everything goes in 4.19-rc2 through Rob's tree that's fine, but if
> > > > > it's queued for 4.20 we might need an immutable tag just in case we
> > > > > queue conflicting changes to the NAND tree.
> > > >
> > > > Ok, thanks.
> > > 
> > > Hi Boris, can you pick this one up. It conflicts with "mtd: rawnand:
> > > atmel: Fix potential NULL pointer dereference"  
> > 
> > Sure, I'll queue it for -rc2.  
> 
> This one hasn't showed up in -next yet, so sending a reminder.

Applied (thanks for the reminder, I had forgotten :-)). It should show
up in -rc4.

Thanks,

Boris


Re: mtd: rawnand: qcom: Namespace prefix some commands

2018-11-18 Thread Boris Brezillon
On Sat, 2018-11-17 at 03:43:27 UTC, Olof Johansson wrote:
> PAGE_READ is used by RISC-V arch code included through mm headers,
> and it makes sense to bring in a prefix on these in the driver.
> 
> drivers/mtd/nand/raw/qcom_nandc.c:153: warning: "PAGE_READ" redefined
>  #define PAGE_READ   0x2
> In file included from include/linux/memremap.h:7,
>  from include/linux/mm.h:27,
>  from include/linux/scatterlist.h:8,
>  from include/linux/dma-mapping.h:11,
>  from drivers/mtd/nand/raw/qcom_nandc.c:17:
> arch/riscv/include/asm/pgtable.h:48: note: this is the location of the 
> previous definition
> 
> Caught by riscv allmodconfig.
> 
> Signed-off-by: Olof Johansson 
> Reviewed-by: Miquel Raynal 

Applied to http://git.infradead.org/linux-mtd.git master, thanks.

Boris


Re: [v2,5/5] mtd: spi-nor: remove unneeded smpt zeroization

2018-11-15 Thread Boris Brezillon
On Fri, 2018-11-09 at 16:56:56 UTC,  wrote:
> The entire smpt array is initialized with data read from sfdp,
> there is no need to init it with zeroes before.
> 
> Signed-off-by: Tudor Ambarus 

Applied to http://git.infradead.org/linux-mtd.git spi-nor/next, thanks.

Boris


Re: [PATCH v2] mtd: change len type from signed to unsigned type

2018-11-15 Thread Boris Brezillon
On Thu, 15 Nov 2018 00:07:10 -0500
Huijin Park  wrote:

> From: "huijin.park" 
> 
> This patch casts the "len" parameter to an unsigned int.

Hm, that's not really what this patch actually. Actually it avoids the
cast from an int to an unsigned int.

> The callers of erase_write() pass the "len" parameter as unsigned int.

That's the important part here. Callers of erase_write() always pass an
unsigned int, so what you're trying to do here is avoid a cast to an
int.

> 
> Signed-off-by: huijin.park 
> ---

You should have a changelog here.

>  drivers/mtd/mtdblock.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
> index a5b1933..b2d5ed1 100644
> --- a/drivers/mtd/mtdblock.c
> +++ b/drivers/mtd/mtdblock.c
> @@ -56,7 +56,7 @@ struct mtdblk_dev {
>   */
>  
>  static int erase_write (struct mtd_info *mtd, unsigned long pos,
> - int len, const char *buf)
> + unsigned int len, const char *buf)
>  {
>   struct erase_info erase;
>   size_t retlen;



Re: [v2, 3/5] mtd: spi-nor: don't overwrite errno in spi_nor_get_map_in_use()

2018-11-15 Thread Boris Brezillon
On Fri, 2018-11-09 at 16:56:52 UTC,  wrote:
> Don't overwrite the errno from spi_nor_read_raw().
> 
> Signed-off-by: Tudor Ambarus 

Applied to http://git.infradead.org/linux-mtd.git mtd/fixes, thanks.

Boris


Re: [v2,1/5] mtd: spi-nor: don't drop sfdp data if optional parsers fail

2018-11-15 Thread Boris Brezillon
On Fri, 2018-11-09 at 16:56:48 UTC,  wrote:
> JESD216C states that just the Basic Flash Parameter Table is mandatory.
> Already defined (or future) additional parameter headers and tables are
> optional.
> 
> Don't drop already collected sfdp data in case an optional table
> parser fails. In case of failing, each optional parser is responsible
> to roll back to the previously known spi_nor data.
> 
> Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> Reported-by: Yogesh Gaur 
> Suggested-by: Boris Brezillon 
> Signed-off-by: Tudor Ambarus 

Applied to http://git.infradead.org/linux-mtd.git mtd/fixes, thanks.

Boris


Re: [v2,2/5] mtd: spi-nor: fix iteration over smpt array

2018-11-15 Thread Boris Brezillon
On Fri, 2018-11-09 at 16:56:50 UTC,  wrote:
> Iterate over smpt array using its starting address and length
> instead of the blind iterations that used data found in the array.
> 
> This prevents possible memory accesses outside of the smpt array
> boundaries in case software, or manufacturers, misrepresent smpt
> array fields.
> 
> Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> Suggested-by: Boris Brezillon 
> Signed-off-by: Tudor Ambarus 

Applied to http://git.infradead.org/linux-mtd.git mtd/fixes, thanks.

Boris


Re: [v2,4/5] mtd: spi_nor: pass DMA-able buffer to spi_nor_read_raw()

2018-11-15 Thread Boris Brezillon
On Fri, 2018-11-09 at 16:56:54 UTC,  wrote:
> spi_nor_read_raw() calls nor->read() which might be implemented
> by the m25p80 driver. m25p80 uses the spi-mem layer which requires
> DMA-able in/out buffers. Pass kmalloc'ed dma buffer to spi_nor_read_raw().
> 
> Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> Signed-off-by: Tudor Ambarus 

Applied to http://git.infradead.org/linux-mtd.git mtd/fixes, thanks.

Boris


Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-15 Thread Boris Brezillon
On Thu, 15 Nov 2018 11:43:05 +
Schrempf Frieder  wrote:

> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
> > Hi Frieder,
> > 
> > With below patch on top of your v5, Read/Write/Erase on CS1 is working fine 
> > for me.  
> 
> Ok, are you sure, that AHB read is working too with this patch?
> You are removing the memmap_phy offset from SFAR and the SFXXAD register 
> values.
> 
> I can understand that selection of the CS and IP commands will work like 
> this, but I can't understand how AHB read should work without the base 
> address of the mapped memory.
> 
> I'm afraid I still don't fully understand the background of these 
> things,

Same here. Yogesh, can you give us more detail on why you decided to
drop the memmap_phy offset?

> but still thank you for testing.
> 


Re: [PATCH v6 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller

2018-11-15 Thread Boris Brezillon
On Thu, 15 Nov 2018 14:04:00 +0100
Miquel Raynal  wrote:

> Hi Liang,
> 
> Liang Yang  wrote on Thu, 15 Nov 2018 19:25:07
> +0800:
> 
> > Hi Boris,
> > 
> > I have implemented dma access base on these helpers you provided below.
> > we prepare to send v7 version now, so when will these helpers be pushed?  
> 
> Thanks for your work. You can send the v7 so we will have a look at the
> overall driver; but since we raised the DMA buffers issue we had a
> discussion with Boris about how to handle them and I think we are going
> to adopt the same solution as Wolfram in the I2C subsystem: manual
> flagging. Sadly, this is probably the best we can do to ensure proper
> DMA support.
> 
> There is nothing set is stone yet but I started a small rework to
> handle MTD operations differently (and add a DMA_SAFE flag), you can
> have a look there [1]. Don't base your work on it for now as it is just
> a preliminary version, subject to big changes.

In order to not block the driver, I'd suggest that you move the helper
I proposed directly into your driver and prefix them with 'meson_'.


Re: [PATCH] mtd: spi-nor: Return error when nor->addr_width not match the device size

2018-11-15 Thread Boris Brezillon
On Thu, 15 Nov 2018 10:54:39 +
 wrote:

> Hi, Liu, Boris, Cyrille,
> 
> On 11/14/2018 03:51 PM, Boris Brezillon wrote:
> > On Wed, 14 Nov 2018 20:56:05 +0800
> > Liu Xiang  wrote:
> >   
> >> In is25lp256, the DWORD1 of JEDEC Basic Flash Parameter Header
> >> is 0xfff920e5. So the DWORD1[18:17] Address Bytes bits are 0b00,  
> 
> Liu, can you point us to a datasheet that has the JEDEC BFPT tables 
> described? I
> couldn't find one ...
> 
> >> means that 3-Byte only addressing.  
> > 
> > According to your other patch this NOR supports 4B opcode, which means
> > the SFDP table is wrong.
> >   
> >> But the device size is larger
> >> than 16MB, nor->addr_width must be 4 to access the whole address.
> >> An error should be returned when nor->addr_width not match  
> > 
> >    ^does not
> >   
> >> the device size in spi_nor_parse_sfdp().
> >>
> >> Suggested-by: Boris Brezillon 
> >> Signed-off-by: Liu Xiang 
> >> ---
> >>  drivers/mtd/spi-nor/spi-nor.c | 4 
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> >> index 3eba13a..77eaf22 100644
> >> --- a/drivers/mtd/spi-nor/spi-nor.c
> >> +++ b/drivers/mtd/spi-nor/spi-nor.c
> >> @@ -2669,6 +2669,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> >>}
> >>params->size >>= 3; /* Convert to bytes. */
> >>  
> >> +  /*if the device exceeds 16MiB, addr_width must be 4*/  
> > 
> > Please add a white space after '/*' and before '*/':
> > 
> > /* If the device exceeds 16MiB, ->addr_width must be 4. */
> >   
> >> +  if ((params->size > 0x100) && (nor->addr_width == 3))  
> > 
> > Parens are not needed around sub-conditions:
> > 
> > if (params->size > 0x100 && nor->addr_width == 3)
> >   
> >> +  return -EINVAL;
> >> +  
> > 
> > I'm not sure this is correct. Looks like some NORs only support 3B
> > opcodes but have a "4-byte addressing" mode (see set_4byte() [1]).
> > Don't know what's reported by the BFPT section in this case though
> > (BFPT_DWORD1_ADDRESS_BYTES_3_ONLY or BFPT_DWORD1_ADDRESS_BYTES_3_OR_4).  
> 
> Boris, this is in close relation with your second patch: [PATCH v3 2/2] mtd:
> spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B.
> 
> When looking again at this, I would say that for the flashes that have a 
> "4-byte
> addressing" mode, but just 3B opcodes, I would expect the DWORD1[18:17] to be 
> of
> value BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (enters 4-Byte mode on command - uses 
> 3B
> opcodes).

The NOR we have and which is exposing BFPT_DWORD1_ADDRESS_BYTES_3_OR_4
actually supports both 3B and 4B commands, so, in this particular case,
BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 does not mean "3B opcode+4-byte
addressing mode"

> 
> If BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 and 4B opcodes, then we can query BFPT
> DWORD16[31:24]: it should have value xx1x_b to indicate that 4B opcodes 
> are
> supported. But which 4B opcodes are supported?

I hope all of them. Wouldn't make sense to have only some of them
supported.

> Do all 3B opcodes have a 4B
> opcode correspondent if SFDP 4-byte table is not available? This might be a 
> good
> assumption, but I can't see it anywhere in jesd216c.

I hope so...


Re: [PATCH -next] i3c: master: Remove set but not used variable 'old_i3c_scl_lim'

2018-11-15 Thread Boris Brezillon
On Wed, 14 Nov 2018 06:10:47 +
YueHaibing  wrote:

> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/i3c/master/i3c-master-cdns.c: In function 'cdns_i3c_master_do_daa':
> drivers/i3c/master/i3c-master-cdns.c:1137:16: warning:
>  variable 'old_i3c_scl_lim' set but not used [-Wunused-but-set-variable]
> 
> It never used since introdution in commit
>   acfab7d324b2 ("i3c: master: Add driver for Cadence IP")
> 
> Signed-off-by: YueHaibing 

Applied.

Thanks,

Boris

> ---
>  drivers/i3c/master/i3c-master-cdns.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/i3c/master/i3c-master-cdns.c 
> b/drivers/i3c/master/i3c-master-cdns.c
> index ad40162..e828921 100644
> --- a/drivers/i3c/master/i3c-master-cdns.c
> +++ b/drivers/i3c/master/i3c-master-cdns.c
> @@ -1133,7 +1133,6 @@ static void cdns_i3c_master_upd_i3c_scl_lim(struct 
> cdns_i3c_master *master)
>  static int cdns_i3c_master_do_daa(struct i3c_master_controller *m)
>  {
>   struct cdns_i3c_master *master = to_cdns_i3c_master(m);
> - unsigned long old_i3c_scl_lim;
>   u32 olddevs, newdevs;
>   int ret, slot;
>   u8 addrs[MAX_DEVS] = { };
> @@ -1165,9 +1164,6 @@ static int cdns_i3c_master_do_daa(struct 
> i3c_master_controller *m)
>   newdevs = readl(master->regs + DEVS_CTRL) & DEVS_CTRL_DEVS_ACTIVE_MASK;
>   newdevs &= ~olddevs;
>  
> - /* Save the old limitation before add devices. */
> - old_i3c_scl_lim = master->i3c_scl_lim;
> -
>   /*
>* Clear all retaining registers filled during DAA. We already
>* have the addressed assigned to them in the addrs array.
> 
> 
> 



Re: [PATCH] mtd: spi-nor: Add 4B_OPCODES flag to is25lp256

2018-11-14 Thread Boris Brezillon
On Wed, 14 Nov 2018 20:55:18 +0800
Liu Xiang  wrote:

> The is25lp256 supports 4-byte opcodes and quad output.
> 
> Suggested-by: Boris Brezillon 
> Signed-off-by: Liu Xiang 

This one looks good. Tudor, can you have a look?

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3e54e31..3eba13a 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1357,7 +1357,7 @@ static int spi_nor_is_locked(struct mtd_info *mtd, 
> loff_t ofs, uint64_t len)
>   { "is25lp128",  INFO(0x9d6018, 0, 64 * 1024, 256,
>   SECT_4K | SPI_NOR_DUAL_READ) },
>   { "is25lp256",  INFO(0x9d6019, 0, 64 * 1024, 512,
> - SECT_4K | SPI_NOR_DUAL_READ) },
> + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | 
> SPI_NOR_4B_OPCODES) },
>   { "is25wp032",  INFO(0x9d7016, 0, 64 * 1024,  64,
>   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>   { "is25wp064",  INFO(0x9d7017, 0, 64 * 1024, 128,



Re: [PATCH] mtd: spi-nor: Return error when nor->addr_width not match the device size

2018-11-14 Thread Boris Brezillon
On Wed, 14 Nov 2018 20:56:05 +0800
Liu Xiang  wrote:

> In is25lp256, the DWORD1 of JEDEC Basic Flash Parameter Header
> is 0xfff920e5. So the DWORD1[18:17] Address Bytes bits are 0b00,
> means that 3-Byte only addressing.

According to your other patch this NOR supports 4B opcode, which means
the SFDP table is wrong.

> But the device size is larger
> than 16MB, nor->addr_width must be 4 to access the whole address.
> An error should be returned when nor->addr_width not match

   ^does not

> the device size in spi_nor_parse_sfdp().
> 
> Suggested-by: Boris Brezillon 
> Signed-off-by: Liu Xiang 
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3eba13a..77eaf22 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2669,6 +2669,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>   }
>   params->size >>= 3; /* Convert to bytes. */
>  
> + /*if the device exceeds 16MiB, addr_width must be 4*/

Please add a white space after '/*' and before '*/':

/* If the device exceeds 16MiB, ->addr_width must be 4. */

> + if ((params->size > 0x100) && (nor->addr_width == 3))

Parens are not needed around sub-conditions:

if (params->size > 0x100 && nor->addr_width == 3)

> + return -EINVAL;
> +

I'm not sure this is correct. Looks like some NORs only support 3B
opcodes but have a "4-byte addressing" mode (see set_4byte() [1]).
Don't know what's reported by the BFPT section in this case though
(BFPT_DWORD1_ADDRESS_BYTES_3_ONLY or BFPT_DWORD1_ADDRESS_BYTES_3_OR_4).

Anyway, I think this check should be moved here [2] to cover the
non-SFDP case.

>   /* Fast Read settings. */
>   for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_reads); i++) {
>   const struct sfdp_bfpt_read *rd = _bfpt_reads[i];

[1]https://elixir.bootlin.com/linux/v4.20-rc2/source/drivers/mtd/spi-nor/spi-nor.c#L278
[2]https://elixir.bootlin.com/linux/v4.20-rc2/source/drivers/mtd/spi-nor/spi-nor.c#L3758


Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Boris Brezillon
On Wed, 14 Nov 2018 10:43:00 +
Yogesh Narayan Gaur  wrote:

> Hi Frieder,
> 
> [..]
> > >
> > > Ok, I will have a look at what could make the chip selection fail in
> > > case of AHB read.  
> > 
> > Could you try with this change applied:
> > 
> > @@ -503,7 +503,7 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
> > struct
> > spi_device *spi)
> >  map_addr = q->memmap_phy;
> >  else
> >  map_addr = q->memmap_phy +
> > -  2 * q->devtype_data->ahb_buf_size;
> > +  q->devtype_data->ahb_buf_size;
> > 
> >  qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD +
> > (i * 4));
> >  }
> >   
> 
> I have tried above change and also have done few more changes but still AHB 
> read for CS1 is falling.

Have plugged a scope on the CS1 line, to make sure it's properly
asserted when the memory is accessed?

> 
> I guess we need to implement dynamic memory mapping [1] for AHB Read as was 
> being done in previous driver implementation.
> Would try this and update you.

Sorry but I don't see why it would solve the problem we have here, but
if it does, I'd like to have a clear explanation ;-).


Re: [PATCH 3/4] drm/v3d: Clean up the reservation object setup.

2018-11-13 Thread Boris Brezillon
On Thu,  8 Nov 2018 08:16:53 -0800
Eric Anholt  wrote:

> The extra to_v3d_bo() calls came from copying this from the vc4
> driver, which stored the cma gem object in the structs.
> 
> Signed-off-by: Eric Anholt 

Reviewed-by: Boris Brezillon 

> ---
>  drivers/gpu/drm/v3d/v3d_gem.c | 32 +++-
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
> index b88c96911453..d0dfdcbbd42c 100644
> --- a/drivers/gpu/drm/v3d/v3d_gem.c
> +++ b/drivers/gpu/drm/v3d/v3d_gem.c
> @@ -214,10 +214,8 @@ v3d_attach_object_fences(struct v3d_exec_info *exec)
>   int i;
>  
>   for (i = 0; i < exec->bo_count; i++) {
> - bo = to_v3d_bo(>bo[i]->base);
> -
>   /* XXX: Use shared fences for read-only objects. */
> - reservation_object_add_excl_fence(bo->resv, out_fence);
> + reservation_object_add_excl_fence(exec->bo[i]->resv, out_fence);
>   }
>  }
>  
> @@ -228,11 +226,8 @@ v3d_unlock_bo_reservations(struct drm_device *dev,
>  {
>   int i;
>  
> - for (i = 0; i < exec->bo_count; i++) {
> - struct v3d_bo *bo = to_v3d_bo(>bo[i]->base);
> -
> - ww_mutex_unlock(>resv->lock);
> - }
> + for (i = 0; i < exec->bo_count; i++)
> + ww_mutex_unlock(>bo[i]->resv->lock);
>  
>   ww_acquire_fini(acquire_ctx);
>  }
> @@ -251,13 +246,13 @@ v3d_lock_bo_reservations(struct drm_device *dev,
>  {
>   int contended_lock = -1;
>   int i, ret;
> - struct v3d_bo *bo;
>  
>   ww_acquire_init(acquire_ctx, _ww_class);
>  
>  retry:
>   if (contended_lock != -1) {
> - bo = to_v3d_bo(>bo[contended_lock]->base);
> + struct v3d_bo *bo = exec->bo[contended_lock];
> +
>   ret = ww_mutex_lock_slow_interruptible(>resv->lock,
>  acquire_ctx);
>   if (ret) {
> @@ -270,19 +265,16 @@ v3d_lock_bo_reservations(struct drm_device *dev,
>   if (i == contended_lock)
>   continue;
>  
> - bo = to_v3d_bo(>bo[i]->base);
> -
> - ret = ww_mutex_lock_interruptible(>resv->lock, acquire_ctx);
> + ret = ww_mutex_lock_interruptible(>bo[i]->resv->lock,
> +   acquire_ctx);
>   if (ret) {
>   int j;
>  
> - for (j = 0; j < i; j++) {
> - bo = to_v3d_bo(>bo[j]->base);
> - ww_mutex_unlock(>resv->lock);
> - }
> + for (j = 0; j < i; j++)
> + ww_mutex_unlock(>bo[j]->resv->lock);
>  
>   if (contended_lock != -1 && contended_lock >= i) {
> - bo = to_v3d_bo(>bo[contended_lock]->base);
> + struct v3d_bo *bo = exec->bo[contended_lock];
>  
>   ww_mutex_unlock(>resv->lock);
>   }
> @@ -303,9 +295,7 @@ v3d_lock_bo_reservations(struct drm_device *dev,
>* before we commit the CL to the hardware.
>*/
>   for (i = 0; i < exec->bo_count; i++) {
> - bo = to_v3d_bo(>bo[i]->base);
> -
> - ret = reservation_object_reserve_shared(bo->resv, 1);
> + ret = reservation_object_reserve_shared(exec->bo[i]->resv, 1);
>   if (ret) {
>   v3d_unlock_bo_reservations(dev, exec, acquire_ctx);
>   return ret;



Re: [PATCH v6 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller

2018-11-12 Thread Boris Brezillon
On Mon, 12 Nov 2018 17:54:16 +0100
Boris Brezillon  wrote:

> +Wolfram to give some inputs on the DMA issue.
> 
> On Mon, 12 Nov 2018 17:13:51 +0100
> Miquel Raynal  wrote:
> 
> > Hello,
> > 
> > Boris Brezillon  wrote on Tue, 6 Nov 2018
> > 11:22:06 +0100:
> >   
> > > On Tue, 6 Nov 2018 18:00:37 +0800
> > > Liang Yang  wrote:
> > > 
> > > > On 2018/11/6 17:28, Boris Brezillon wrote:  
> > > > > On Tue, 6 Nov 2018 17:08:00 +0800
> > > > > Liang Yang  wrote:
> > > > > 
> > > > >> On 2018/11/5 23:53, Boris Brezillon wrote:
> > > > >>> On Fri, 2 Nov 2018 00:42:21 +0800
> > > > >>> Jianxin Pan  wrote:
> > > > >>>
> > > > >>>> +
> > > > >>>> +static inline u8 meson_nfc_read_byte(struct mtd_info *mtd)
> > > > >>>> +{
> > > > >>>> +  struct nand_chip *nand = mtd_to_nand(mtd);
> > > > >>>> +  struct meson_nfc *nfc = nand_get_controller_data(nand);
> > > > >>>> +  u32 cmd;
> > > > >>>> +
> > > > >>>> +  cmd = nfc->param.chip_select | NFC_CMD_DRD | 0;
> > > > >>>> +  writel(cmd, nfc->reg_base + NFC_REG_CMD);
> > > > >>>> +
> > > > >>>> +  meson_nfc_drain_cmd(nfc);
> > > > >>>
> > > > >>> You probably don't want to drain the FIFO every time you read a 
> > > > >>> byte on
> > > > >>> the bus, and I guess the INPUT FIFO is at least as big as the CMD
> > > > >>> FIFO, right? If that's the case, you should queue as much DRD cmd as
> > > > >>> possible and only sync when the user explicitly requests it or when
> > > > >>> the INPUT/READ FIFO is full.
> > > > >>>
> > > > >> Register 'NFC_REG_BUF' can holds only 4 bytes, also DRD sends only 
> > > > >> one
> > > > >> nand cycle to read one byte and covers the 1st byte every time 
> > > > >> reading.
> > > > >> i think nfc controller is faster than nand cycle, but really it is 
> > > > >> not
> > > > >> high efficiency when reading so many bytes once.
> > > > >> Or use dma command here like read_page and read_page_raw.
> > > > > 
> > > > > Yep, that's also an alternative, though you'll have to make sure the
> > > > > buffer passed through the nand_op_inst is DMA-safe, and use a bounce
> > > > > buffer when that's not the case.
> > > > > 
> > > > ok, i will try dma here.  
> > > 
> > > We should probably expose the bounce buf handling as generic helpers at
> > > the rawnand level:
> > > 
> > > void *nand_op_get_dma_safe_input_buf(struct nand_op_instr *instr)
> > > {
> > >   void *buf;
> > > 
> > >   if (WARN_ON(instr->type != NAND_OP_DATA_IN_INSTR))
> > >   return NULL;
> > > 
> > >   if (virt_addr_valid(instr->data.in) &&
> > >   !object_is_on_stack(instr->data.buf.in))
> > >   return instr->data.buf.in;
> > > 
> > >   return kzalloc(instr->data.len, GFP_KERNEL);
> > > }
> > > 
> > > void nand_op_put_dma_safe_input_buf(struct nand_op_instr *instr,
> > >   void *buf)
> > > {
> > >   if (WARN_ON(instr->type != NAND_OP_DATA_IN_INSTR) ||
> > >   WARN_ON(!buf))
> > >   return;
> > > 
> > >   if (buf == instr->data.buf.in)
> > >   return;
> > > 
> > >   memcpy(instr->data.buf.in, buf, instr->data.len);
> > >   kfree(buf);
> > > }
> > > 
> > > const void *nand_op_get_dma_safe_output_buf(struct nand_op_instr *instr)
> > > {
> > >   void *buf;
> > > 
> > >   if (WARN_ON(instr->type != NAND_OP_DATA_OUT_INSTR))
> > >   return NULL;
> > > 
> > >   if (virt_addr_valid(instr->data.out) &&
> > >   !object_is_on_stack(instr->data.buf.out))
> > >   return instr->data.buf.out;
> > > 
> > >   return kmemdup(instr->data.buf.out, GFP_KERNEL);
> > > }
> > > 
> > > void nand_op_put_dma_safe_outpu

Re: [PATCH v6 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller

2018-11-12 Thread Boris Brezillon
+Wolfram to give some inputs on the DMA issue.

On Mon, 12 Nov 2018 17:13:51 +0100
Miquel Raynal  wrote:

> Hello,
> 
> Boris Brezillon  wrote on Tue, 6 Nov 2018
> 11:22:06 +0100:
> 
> > On Tue, 6 Nov 2018 18:00:37 +0800
> > Liang Yang  wrote:
> >   
> > > On 2018/11/6 17:28, Boris Brezillon wrote:
> > > > On Tue, 6 Nov 2018 17:08:00 +0800
> > > > Liang Yang  wrote:
> > > >   
> > > >> On 2018/11/5 23:53, Boris Brezillon wrote:  
> > > >>> On Fri, 2 Nov 2018 00:42:21 +0800
> > > >>> Jianxin Pan  wrote:
> > > >>>  
> > > >>>> +
> > > >>>> +static inline u8 meson_nfc_read_byte(struct mtd_info *mtd)
> > > >>>> +{
> > > >>>> +struct nand_chip *nand = mtd_to_nand(mtd);
> > > >>>> +struct meson_nfc *nfc = nand_get_controller_data(nand);
> > > >>>> +u32 cmd;
> > > >>>> +
> > > >>>> +cmd = nfc->param.chip_select | NFC_CMD_DRD | 0;
> > > >>>> +writel(cmd, nfc->reg_base + NFC_REG_CMD);
> > > >>>> +
> > > >>>> +meson_nfc_drain_cmd(nfc);  
> > > >>>
> > > >>> You probably don't want to drain the FIFO every time you read a byte 
> > > >>> on
> > > >>> the bus, and I guess the INPUT FIFO is at least as big as the CMD
> > > >>> FIFO, right? If that's the case, you should queue as much DRD cmd as
> > > >>> possible and only sync when the user explicitly requests it or when
> > > >>> the INPUT/READ FIFO is full.
> > > >>>  
> > > >> Register 'NFC_REG_BUF' can holds only 4 bytes, also DRD sends only one
> > > >> nand cycle to read one byte and covers the 1st byte every time reading.
> > > >> i think nfc controller is faster than nand cycle, but really it is not
> > > >> high efficiency when reading so many bytes once.
> > > >> Or use dma command here like read_page and read_page_raw.  
> > > > 
> > > > Yep, that's also an alternative, though you'll have to make sure the
> > > > buffer passed through the nand_op_inst is DMA-safe, and use a bounce
> > > > buffer when that's not the case.
> > > >   
> > > ok, i will try dma here.
> > 
> > We should probably expose the bounce buf handling as generic helpers at
> > the rawnand level:
> > 
> > void *nand_op_get_dma_safe_input_buf(struct nand_op_instr *instr)
> > {
> > void *buf;
> > 
> > if (WARN_ON(instr->type != NAND_OP_DATA_IN_INSTR))
> > return NULL;
> > 
> > if (virt_addr_valid(instr->data.in) &&
> > !object_is_on_stack(instr->data.buf.in))
> > return instr->data.buf.in;
> > 
> > return kzalloc(instr->data.len, GFP_KERNEL);
> > }
> > 
> > void nand_op_put_dma_safe_input_buf(struct nand_op_instr *instr,
> > void *buf)
> > {
> > if (WARN_ON(instr->type != NAND_OP_DATA_IN_INSTR) ||
> > WARN_ON(!buf))
> > return;
> > 
> > if (buf == instr->data.buf.in)
> > return;
> > 
> > memcpy(instr->data.buf.in, buf, instr->data.len);
> > kfree(buf);
> > }
> > 
> > const void *nand_op_get_dma_safe_output_buf(struct nand_op_instr *instr)
> > {
> > void *buf;
> > 
> > if (WARN_ON(instr->type != NAND_OP_DATA_OUT_INSTR))
> > return NULL;
> > 
> > if (virt_addr_valid(instr->data.out) &&
> > !object_is_on_stack(instr->data.buf.out))
> > return instr->data.buf.out;
> > 
> > return kmemdup(instr->data.buf.out, GFP_KERNEL);
> > }
> > 
> > void nand_op_put_dma_safe_output_buf(struct nand_op_instr *instr,
> > void *buf)
> > {
> > if (WARN_ON(instr->type != NAND_OP_DATA_OUT_INSTR) ||
> > WARN_ON(!buf))
> > return;
> > 
> > if (buf != instr->data.buf.out)
> > kfree(buf);
> > }  
> 
> Not that I am against such function, but maybe they should come with
> comments stating that there is no reliable way to find if a buffer is
> DMA-able at runtime and these are just sanity checks (ie. required, but
> probably not enough).

It'

Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller

2018-11-12 Thread Boris Brezillon
On Mon, 12 Nov 2018 11:55:36 +0100
Martin Lund  wrote:

> Hi Naga,
> 
> Just a few review comments for the v12 version.
> 
> On Fri, Nov 9, 2018 at 6:00 AM Naga Sureshkumar Relli
>  wrote:
> > +#define PKT_OFST   0x00
> > +#define PKT_CNT_SHIFT  12
> > +
> > +#define MEM_ADDR1_OFST 0x04
> > +#define MEM_ADDR2_OFST 0x08  
> 
> For the sake of readability I think *_OFFSET is preferred, especially
> since the driver already includes short macro names. I think this is
> similar to the EVNT vs EVENT point.
> The majority of drivers in the Linux kernel do not shorten OFFSET -> OFST.
> 
> 
> > +static void anfc_rw_pio_op(struct mtd_info *mtd, u8 *buf, int len,
> > +  bool do_read, int prog, int pktcount, int 
> > pktsize)
> > +{
> > +   struct nand_chip *chip = mtd_to_nand(mtd);
> > +   struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> > +   struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > +   u32 *bufptr = (u32 *)buf;
> > +   u32 cnt = 0, intr = 0;
> > +
> > +   anfc_config_dma(nfc, 0);
> > +
> > +   if (pktsize == 0)
> > +   pktsize = len;
> > +
> > +   anfc_setpktszcnt(nfc, pktsize, pktcount);
> > +
> > +   if (!achip->strength)
> > +   intr = MBIT_ERROR;
> > +
> > +   if (do_read)
> > +   intr |= READ_READY;
> > +   else
> > +   intr |= WRITE_READY;
> > +   anfc_enable_intrs(nfc, intr);
> > +   writel(prog, nfc->base + PROG_OFST);
> > +   while (cnt < pktcount) {
> > +   anfc_wait_for_event(nfc);
> > +   cnt++;
> > +   if (cnt == pktcount)
> > +   anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +   if (do_read)
> > +   ioread32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > +pktsize / 4);
> > +   else
> > +   iowrite32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > + pktsize / 4);
> > +   bufptr += (pktsize / 4);
> > +   if (cnt < pktcount)
> > +   anfc_enable_intrs(nfc, intr);
> > +   }
> > +   anfc_wait_for_event(nfc);
> > +}  
> 
> Throughout the driver all calls to anfc_wait_for_event() ignores the
> timeout return value. It would be nice to see some error handling in
> case it times out - at minimum consider printing out an error message
> since timeout on NAND operations are fairly critical and should
> generally not occur. Perhaps even an argument can be made for
> returning -ETIMEDOUT in case of timeout.

Yes please, check anfc_wait_for_event() return code and propagate the
error to the upper layer.


Re: [PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework

2018-11-12 Thread Boris Brezillon
On Mon, 12 Nov 2018 10:46:45 +
Schrempf Frieder  wrote:

> On 08.11.18 09:34, Boris Brezillon wrote:
> > On Wed, 7 Nov 2018 16:36:13 +
> > Schrempf Frieder  wrote:
> >   
> >> Hi Olof,
> >>
> >> On 07.11.18 17:20, Olof Johansson wrote:  
> >>> On Wed, Nov 7, 2018 at 6:44 AM Frieder Schrempf
> >>>  wrote:  
> >>>>
> >>>> From: Frieder Schrempf 
> >>>>
> >>>> The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver
> >>>> at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs.
> >>>>
> >>>> Signed-off-by: Frieder Schrempf   
> >>>
> >>> Hi Frieder,
> >>>
> >>> This patch is part of a series that I didn't see the rest of, but in
> >>> general we prefer to merge these through arm-soc even if the driver
> >>> goes in through another tree. The way we'd prefer to handle it is that
> >>> once the driver lands, we'll take the config option change to turn it
> >>> on. To avoid our branches to break until both sides have landed, it
> >>> might be a good idea to keep both drivers on for a short while (one
> >>> release).
> >>>
> >>> So, I'm not going to ack this since we avoid taking defconfig changes
> >>> through driver trees (these two defconfigs tend to churn a lot and we
> >>> don't want to create merge conflicts where we don't have to), but
> >>> we'll be happy to pick it up when the time comes.  
> >>
> >> Ok, thank you for explaining the common practice. I will drop the config
> >> changes for the next version and send it separately when the time is ready.
> >>
> >> Both the old driver and the new one use the same compatible strings for
> >> probing. Wouldn't that cause problems if both drivers are enabled for a
> >> while, or am I missing something?  
> > 
> > Or maybe we should not introduce a new Kconfig option and just reuse
> > the old one. It probably requires re-ordering patches a bit (patch 1
> > should be moved after patch 5). Then you have 2 choices:
> > 
> > 1/ merge patch 1 and 6 so that the new driver effectively replaces the
> > old one but uses the same Kconfig option
> > 2/ remove the ability to compile the old driver when the new one is
> > introduced: remove the line from drivers/mtd/spi-nor Makefile and
> > move the Kconfig entry from drivers/mtd/spi-nor/Kconfig to
> > drivers/spi/Kconfig. And remove the old code in a separate patch
> > 
> > I'm fine either way, but option #2 will probably make the patch
> > introducing the new driver bigger and hurt readability.  
> 
> I think having both drivers in the tree for a while wouldn't be so bad. 
> So if any compatibility issues come up with the new driver, people can 
> still use the old one.

Except that's not what happens in practice. Believe me, I tried this
approach several times, and people keep using the old driver until
they're forced to switch to the new one. So you actually don't address
the problem, you just delay it a bit, and you'll have to fix
regressions anyway.

> 
> Therefore I think I will drop the patches that change the defconfig and 
> remove the old driver code and keep the different Kconfig options. And 
> maybe add an exclusive dependency in Kconfig, so both drivers can not be 
> enabled at the same time.
> 
> Does this make sense?

I'd really prefer to have the removal of the old driver in the same
release the new driver is introduced but if that's not possible, let's
have a clear plan, like "introduce new driver in release X, remove the
old one in release X+1".


Re: [PATCH] mtd: spi-nor: Add 4-byte address support for is25lp256

2018-11-11 Thread Boris Brezillon
Hi Liu,

On Fri, 24 Aug 2018 22:41:41 +0800
Liu Xiang  wrote:

> The is25lp256 supports 4-byte opcodes and quad output.
> In is25lp256, the DWORD1 of JEDEC Basic Flash Parameter Header
> is 0xfff920e5. So the DWORD1[18:17] Address Bytes bits are 0b00,
> means that 3-Byte only addressing. Now this limits nor->addr_width
> to 3 and makes it inpossible to access the address above 16MB.
> I think the size of flash is the most important judgement for
> nor->addr_width. Once the size is larger than 16MB, nor->addr_width
> must be 4. This can avoid the bad situation that manufacturer sets
> incorrect value of register.

Please split this patch:
- Add 4B_OPCODES flag to is25lp256
- Rework the ->addr_width selection logic (more about that below).

> 
> Signed-off-by: Liu Xiang 
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d9c368c..0203b09 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1065,7 +1065,7 @@ static int spi_nor_is_locked(struct mtd_info *mtd, 
> loff_t ofs, uint64_t len)
>   { "is25lp128",  INFO(0x9d6018, 0, 64 * 1024, 256,
>   SECT_4K | SPI_NOR_DUAL_READ) },
>   { "is25lp256",  INFO(0x9d6019, 0, 64 * 1024, 512,
> - SECT_4K | SPI_NOR_DUAL_READ) },
> + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | 
> SPI_NOR_4B_OPCODES) },
>   { "is25wp032",  INFO(0x9d7016, 0, 64 * 1024,  64,
>   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>   { "is25wp064",  INFO(0x9d7017, 0, 64 * 1024, 128,
> @@ -2926,16 +2926,16 @@ int spi_nor_scan(struct spi_nor *nor, const char 
> *name,
>   if (ret)
>   return ret;
>  
> - if (nor->addr_width) {
> - /* already configured from SFDP */
> - } else if (info->addr_width) {
> - nor->addr_width = info->addr_width;
> - } else if (mtd->size > 0x100) {
> + if (mtd->size > 0x100) {
>   /* enable 4-byte addressing if the device exceeds 16MiB */
>   nor->addr_width = 4;
>   if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> - info->flags & SPI_NOR_4B_OPCODES)
> + info->flags & SPI_NOR_4B_OPCODES)
>   spi_nor_set_4byte_opcodes(nor, info);
> + } else if (nor->addr_width) {
> + /* already configured from SFDP */
> + } else if (info->addr_width) {
> + nor->addr_width = info->addr_width;
>   } else {
>   nor->addr_width = 3;
>   }

We'd rather return an error and warn users when nor->addr_width does not
match the device size than fix this mismatch silently.

Regards,

Boris


Re: mtd: cfi_cmdset_0020: Mark expected switch fall-throughs

2018-11-09 Thread Boris Brezillon
On Wed, 2018-08-15 at 17:02:26 UTC, "Gustavo A. R. Silva" wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Addresses-Coverity-ID: 114857 ("Missing break in switch")
> Addresses-Coverity-ID: 114858 ("Missing break in switch")
> Addresses-Coverity-ID: 114859 ("Missing break in switch")
> Addresses-Coverity-ID: 114860 ("Missing break in switch")
> Addresses-Coverity-ID: 114861 ("Missing break in switch")
> Addresses-Coverity-ID: 114862 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva 

Applied to http://git.infradead.org/linux-mtd.git mtd/next, thanks.

Boris


Re: mtd: block2mtd: mark expected switch fall-throughs

2018-11-09 Thread Boris Brezillon
On Thu, 2018-08-09 at 16:05:13 UTC, "Gustavo A. R. Silva" wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Addresses-Coverity-ID: 402015 ("Missing break in switch")
> Addresses-Coverity-ID: 402016 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva 

Applied to http://git.infradead.org/linux-mtd.git mtd/next, thanks.

Boris


Re: mtd: Kconfig: fix spelling mistake "partions" -> "partition"

2018-11-09 Thread Boris Brezillon
On Tue, 2018-09-11 at 12:42:34 UTC, Colin King wrote:
> From: Colin Ian King 
> 
> Trivial fix to spelling mistake in the Kconfig text
> 
> Signed-off-by: Colin Ian King 

Applied to http://git.infradead.org/linux-mtd.git mtd/next, thanks.

Thanks!


Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller

2018-11-09 Thread Boris Brezillon
Hi Naga,

Just a preliminary review. I still think we have problems with how you
execute NAND operations. You seem to assume that read/write operation
are always page write/read operation with a size aligned on a page
size. This is wrong, either the controller is able to execute the exact
operation that has been requested or it returns -ENOTSUPP.

On Fri, 9 Nov 2018 10:30:41 +0530
Naga Sureshkumar Relli  wrote:

> +
> +/**
> + * struct anfc_nand_chip - Defines the nand chip related information
> + * @node:Used to store NAND chips into a list.
> + * @chip:NAND chip information structure.
> + * @strength:Bch or Hamming mode enable/disable.

The name is still confusing. BTW, can't you deduce Hamming vs BCH from
the strength? Hamming strength is 1, while BCH strengths seem to start
at 4.

> + * @ecc_strength:Ecc strength 4.8/12/16.

  ^/

> + * @eccval:  Ecc config value.
> + * @spare_caddr_cycles:  Column address cycle information for spare area.
> + * @pktsize: Packet size for read / write operation.
> + * @csnum:   chipselect number to be used.
> + * @spktsize:Packet size in ddr mode for status operation.
> + * @inftimeval:  Data interface and timing mode information
> + */
> +struct anfc_nand_chip {
> + struct list_head node;
> + struct nand_chip chip;
> + bool strength;
> + u32 ecc_strength;
> + u32 eccval;
> + u16 spare_caddr_cycles;
> + u32 pktsize;
> + int csnum;
> + u32 spktsize;
> + u32 inftimeval;
> +};
> +
> +/**
> + * struct anfc_nand_controller - Defines the Arasan NAND flash controller
> + *driver instance
> + * @controller:  base controller structure.
> + * @chips:   list of all nand chips attached to the ctrler.
> + * @dev: Pointer to the device structure.
> + * @base:Virtual address of the NAND flash device.
> + * @clk_sys: Pointer to the system clock.
> + * @clk_flash:   Pointer to the flash clock.
> + * @dma: Dma enable/disable.
> + * @buf: Buffer used for read/write byte operations.
> + * @irq: irq number

You should need this field. Just get the irq num in you probe path an
register an irq handler with devm_request_irq().

> + * @bufshift:Variable used for indexing buffer operation
> + * @csnum:   Chip select number currently inuse.

 ^ in use

> + * @event:   Completion event for nand status events.
> + * @status:  Status of the flash device.
> + * @prog:Used to initiate controller operations.
> + */
> +struct anfc_nand_controller {
> + struct nand_controller controller;
> + struct list_head chips;
> + struct device *dev;
> + void __iomem *base;
> + struct clk *clk_sys;
> + struct clk *clk_flash;
> + int irq;
> + int csnum;
> + struct completion event;
> + int status;
> + u8 buf[TEMP_BUF_SIZE];

Allocate this buf dynamically.

> + u32 eccval;
> +};

> +static int anfc_page_write_type_exec(struct nand_chip *chip,
> +  const struct nand_subop *subop)
> +{
> + const struct nand_op_instr *instr;
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + unsigned int op_id, len;
> + struct anfc_op nfc_op = {};
> + struct mtd_info *mtd = nand_to_mtd(chip);
> +
> + anfc_parse_instructions(chip, subop, _op);
> + instr = nfc_op.data_instr;
> + op_id = nfc_op.data_instr_idx;
> + anfc_prepare_cmd(chip, nfc_op.cmds[0], nfc_op.cmds[1], 1,
> +  mtd->writesize, nfc_op.naddrcycles);
> + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> + if (!nfc_op.data_instr)
> + return 0;
> +
> + len = nand_subop_get_data_len(subop, op_id);
> + anfc_write_data_op(chip,  (char *)instr->ctx.data.buf.out,

^ extra white space here
and please drop the cast.

Can you please run checkpatch --strict prior to submitting patches?

> +mtd->writesize,
> +DIV_ROUND_UP(mtd->writesize, achip->pktsize),

No, that's wrong. You should use instr->ctx.data.len here, and the
DIV_ROUND_UP() thing is a bit scary. That means you might be writing
more data than requested.

> +achip->pktsize);
> +
> + return 0;
> +}
> +

> +
> +static int anfc_probe(struct platform_device *pdev)
> +{
> + struct anfc_nand_controller *nfc;
> + struct anfc_nand_chip *anand_chip;
> + struct device_node *np = pdev->dev.of_node, *child;
> + struct resource *res;
> + int err;
> +
> + nfc = devm_kzalloc(>dev, sizeof(*nfc), GFP_KERNEL);
> + if (!nfc)
> + return -ENOMEM;
> +
> + 

Re: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding documentation

2018-11-08 Thread Boris Brezillon
On Fri, 9 Nov 2018 10:30:39 +0530
Naga Sureshkumar Relli  wrote:

> This patch adds the dts binding document for arasan nand flash controller
> 
> Signed-off-by: Naga Sureshkumar Relli 
> ---
> Changes in v12:
>  - Removed interrupt-parent description as it is implied as suggested by
>Rob Herring
>  - Added missing ';' as required
> Changes in v11:
>  - Updated compatible description as suggested by Boris
>  - Removed arasan-has-dma property
> Changes in v10:
>  - None
> Changes in v9:
>  - None
> Changes in v8:
>  - Updated compatible and clock-names as per Boris comments
> Changes in v7:
>  - Corrected the acronyms those should be in caps
> Changes in v6:
>  - Removed num-cs property
>  - Separated nandchip from nand controller
> Changes in v5:
>  - None
> Changes in v4:
>  - Added num-cs property
>  - Added clock support
> Changes in v3:
>  - None
> Changes in v2:
>  - None
> ---
>  .../devicetree/bindings/mtd/arasan_nand.txt| 32 
> ++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/arasan_nand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/arasan_nand.txt 
> b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> new file mode 100644
> index 000..b522daf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> @@ -0,0 +1,32 @@
> +Arasan NAND Flash Controller with ONFI 3.1 support
> +
> +Required properties:
> +- compatible:Should be "xlnx,zynqmp-nand", "arasan,nfc-v3p10"
> +- reg:   Memory map for module access
> +- interrupts:Should contain the interrupt for the device
> +- clock-name:List of input clocks - "sys", "flash"
> + (See clock bindings for details)
> +- clocks:Clock phandles (see clock bindings for details)
> +
> +Required properties for child node:
> +- nand-ecc-mode: see nand.txt

Why is it required? Can't you fallback to HW when this prop is missing?
Oh, and reg is not listed in the required props.

> +
> +For NAND partition information please refer the below file
> +Documentation/devicetree/bindings/mtd/partition.txt
> +
> +Example:
> + nfc: nand@ff10 {
> + compatible = "xlnx,zynqmp-nand", "arasan,nfc-v3p10";
> + reg = <0x0 0xff10 0x1000>;
> + clock-name = "sys", "flash";
> + clocks = <_clk _clk>;
> + interrupt-parent = <>;
> + interrupts = <0 14 4>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + nand@0 {
> + reg = <0>;
> + nand-ecc-mode = "hw";
> + };
> + };



Re: [PATCH 3/7] mtd: spi-nor: add restriction for nmaps in smpt parser

2018-11-08 Thread Boris Brezillon
On Thu, 8 Nov 2018 14:48:11 +
 wrote:
  
>  +
> >>>
> >>> Maybe I missed something but it sounds like this change is just
> >>> optimizing the SPMT parsing a bit, and to be honest, I'm not sure this
> >>> is really needed. Most of the time, smpt_len will be rather small, so
> >>> trying to bail out earlier is not bringing much perf improvements.
> >>
> >> It's rather a smtp validity check. I want to return an error if there are 
> >> not
> >> enough detection commands to identify the map id.  
> > 
> > You would have failed the same way without this validity check after a
> > maximum of smpt_len iterations, right?
> >   
> 
> Right. The correct fix would be to count nmaps in a loop, then do these 
> checks,
> and if all ok, search for the map_id in another loop :).

Or just error out when !ncmds && nmaps > 1.

If you insist on keeping the ncmds && nmaps >= (1 << (ncmds + 1))
check, that's fine, but it's not replacing the consistency check I was
doing ;-).


Re: [PATCH 2/2] ubi: Expose the bitrot interface

2018-11-08 Thread Boris Brezillon
On Wed,  7 Nov 2018 23:16:19 +0100
Richard Weinberger  wrote:

> +/**
> + * ubi_bitflip_check - Check an eraseblock for bitflips and scrub it if 
> needed.
> + * @ubi: UBI device description object
> + * @pnum: the physical eraseblock to schedule
> + * @force_scrub: force scrubbing if non-zero, schedule erase otherwise

Are you sure about the "schedule erase otherwise"? I'd say force_scrub
only influence when the scrub operation is done: either unconditionally
or depending on the result of ubi_io_read().

> + *
> + * This function reads the given eraseblock and checks if bitflips occured.
> + * In case of bitflips, the eraseblock is scheduled for scrubbing.
> + * If scrubbing is forced with @force_scrub, the eraseblock is not read,
> + * but scheduled for scrubbing right away.
> + *
> + * Returns:
> + * %EINVAL, PEB is out of range
> + * %ENOENT, PEB is no longer used by UBI
> + * %EBUSY, PEB cannot be checked now or a check is currently running on it
> + * %EAGAIN, bit flips happened but scrubbing is currently not possible
> + * %EUCLEAN, bit flips happened and PEB is scheduled for scrubbing
> + * %0, no bit flips detected
> + */
> +int ubi_bitflip_check(struct ubi_device *ubi, int pnum, int force_scrub)



Re: [PATCH 3/7] mtd: spi-nor: add restriction for nmaps in smpt parser

2018-11-08 Thread Boris Brezillon
On Thu, 8 Nov 2018 13:58:45 +
 wrote:

> On 11/08/2018 02:54 PM, Boris Brezillon wrote:
> > On Thu, 8 Nov 2018 11:07:11 +
> >  wrote:
> >   
> >> The map selector is limited to a maximum of 8 bits, allowing
> >> for a maximum of 256 possible map configurations. The total
> >> number of map configurations should be addressable by the
> >> total number of bits described by the detection commands.
> >>
> >> For example: if there are five to eight possible sector map
> >> configurations, at least three configuration detection commands
> >> will be needed to extract three bits of configuration selection
> >> information from the device in order to identify which configuration
> >> is currently in use.
> >>
> >> Suggested-by: Boris Brezillon   
> > 
> > I don't remember suggesting this :-).  
> 
> I see this when you discussed with Yogesh:
> 
> +   for (nmaps = 0; i< smpt_len; nmaps++) {
> +   if(!(smpt[i] & SMPT_DESC_TYPE_MAP)) {
> +   i += 2;
> +   continue;
> +   }
> +
> +   if(!map_id_is_valid) {
> +   if (nmaps) {
> +   ret = NULL;
> +   break;
> +   }
> 
> If I understand correctly, you meant that if there are no detection commands,
> and more than a configuration map, then return an error.

Yes, because in this case we have no way to know what config is
currently selected.

> 
> This is correct in my opinion. What I did was to generalize this idea. If smtp
> defines more maps than you can select using detection commands, return error.

AFAICT you're no longer checking that map_id is valid (see below).

> 
> +
> +   ret = smpt+i;
> +   } else if (map_id == SMPT_MAP_ID(smpt[i])) {
> +   ret = smpt+i;
> +   break;
> +   }
> +
> /* increment the table index to the next map */
> i += SMPT_MAP_REGION_COUNT(smpt[i]) + 1;
> }
> 
> >   
> >> Signed-off-by: Tudor Ambarus 
> >> ---
> >>  drivers/mtd/spi-nor/spi-nor.c | 15 +--
> >>  1 file changed, 13 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> >> index 59dcedb08691..bd1866d714f2 100644
> >> --- a/drivers/mtd/spi-nor/spi-nor.c
> >> +++ b/drivers/mtd/spi-nor/spi-nor.c
> >> @@ -2868,7 +2868,7 @@ static const u32 *spi_nor_get_map_in_use(struct 
> >> spi_nor *nor, const u32 *smpt,
> >>const u32 *ret = NULL;
> >>u32 addr;
> >>int err;
> >> -  u8 i;
> >> +  u8 i, ncmds, nmaps;
> >>u8 addr_width, read_opcode, read_dummy;
> >>u8 read_data_mask, data_byte, map_id;
> >>  
> >> @@ -2877,6 +2877,7 @@ static const u32 *spi_nor_get_map_in_use(struct 
> >> spi_nor *nor, const u32 *smpt,
> >>read_opcode = nor->read_opcode;
> >>  
> >>map_id = 0;
> >> +  ncmds = 0;
> >>/* Determine if there are any optional Detection Command Descriptors */
> >>for (i = 0; i < smpt_len; i += 2) {
> >>if (smpt[i] & SMPT_DESC_TYPE_MAP)
> >> @@ -2896,6 +2897,7 @@ static const u32 *spi_nor_get_map_in_use(struct 
> >> spi_nor *nor, const u32 *smpt,
> >> * Configuration that is currently in use.
> >> */
> >>map_id = map_id << 1 | !!(data_byte & read_data_mask);
> >> +  ncmds++;
> >>}
> >>  
> >>/*
> >> @@ -2905,7 +2907,16 @@ static const u32 *spi_nor_get_map_in_use(struct 
> >> spi_nor *nor, const u32 *smpt,
> >> *
> >> * Find the matching configuration map.
> >> */
> >> -  while (i < smpt_len) {
> >> +  for (nmaps = 0; i < smpt_len; nmaps++) {
> >> +  /*
> >> +   * The map selector is limited to a maximum of 8 bits, allowing
> >> +   * for a maximum of 256 possible map configurations. The total
> >> +   * number of map configurations should be addressable by the
> >> +   * total number of bits described by the detection commands.
> >> +   */
> >> +  if (ncmds && nmaps >= (1 << (ncmds + 1)))
> >> +  break;

You're no longer checking that when ncmds == 0 nmaps has to be 1. So,
say the chip exposed no smpt commands and has several maps defined,
map_id will be 0 while it should have be "undefined". My version
would return an error, but yours tries to find map_id 0.

> >> +  
> > 
> > Maybe I missed something but it sounds like this change is just
> > optimizing the SPMT parsing a bit, and to be honest, I'm not sure this
> > is really needed. Most of the time, smpt_len will be rather small, so
> > trying to bail out earlier is not bringing much perf improvements.  
> 
> It's rather a smtp validity check. I want to return an error if there are not
> enough detection commands to identify the map id.

You would have failed the same way without this validity check after a
maximum of smpt_len iterations, right?


Re: [PATCH 3/7] mtd: spi-nor: add restriction for nmaps in smpt parser

2018-11-08 Thread Boris Brezillon
On Thu, 8 Nov 2018 13:54:47 +0100
Boris Brezillon  wrote:

> > -   while (i < smpt_len) {
> > +   for (nmaps = 0; i < smpt_len; nmaps++) {
> > +   /*
> > +* The map selector is limited to a maximum of 8 bits, allowing
> > +* for a maximum of 256 possible map configurations. The total
> > +* number of map configurations should be addressable by the
> > +* total number of bits described by the detection commands.
> > +*/
> > +   if (ncmds && nmaps >= (1 << (ncmds + 1)))
> > +   break;
> > +  
> 
> Maybe I missed something but it sounds like this change is just
> optimizing the SPMT parsing a bit, and to be honest, I'm not sure this
> is really needed. Most of the time, smpt_len will be rather small, so
> trying to bail out earlier is not bringing much perf improvements.

To make it clearer, I don't think the extra complexity is worth the tiny
perf improvement.


Re: [PATCH 6/7] mtd: spi-nor: ensure memory used for nor->read() is DMA safe

2018-11-08 Thread Boris Brezillon
On Thu, 8 Nov 2018 11:07:18 +
 wrote:

> Use GFP_DMA to ensure that the memory we allocate for transfers in
> nor->read() can be DMAed.

See my comment on patch 5.


Re: [PATCH 5/7] mtd: spi_nor: pass DMA-able buffer to spi_nor_read_raw()

2018-11-08 Thread Boris Brezillon
On Thu, 8 Nov 2018 11:07:16 +
 wrote:

> spi_nor_read_raw() calls nor->read() which might be implemented
> by the m25p80 driver. m25p80 uses the spi-mem layer which requires
> DMA-able in/out buffers. Pass kmalloc'ed dma buffer to spi_nor_read_raw().
> 
> Signed-off-by: Tudor Ambarus 
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 79ca1102faed..2eaa21c85483 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2161,7 +2161,7 @@ spi_nor_set_pp_settings(struct spi_nor_pp_command *pp,
>   * @nor: pointer to a 'struct spi_nor'
>   * @addr:offset in the serial flash memory
>   * @len: number of bytes to read
> - * @buf: buffer where the data is copied into
> + * @buf: buffer where the data is copied into (dma-safe memory)
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> @@ -2868,11 +2868,16 @@ static const u32 *spi_nor_get_map_in_use(struct 
> spi_nor *nor, const u32 *smpt,
>u8 smpt_len)
>  {
>   const u32 *ret;
> + u8 *dma_safe;

I'd prefer to have it named buf, data_buf or scratch_buf...

>   u32 addr;
>   int err;
>   u8 i, ncmds, nmaps;
>   u8 addr_width, read_opcode, read_dummy;
> - u8 read_data_mask, data_byte, map_id;
> + u8 read_data_mask, map_id;
> +

... and have a comment here explaining why your allocating the buffer
instead of using a local var placed on the stack.

> + dma_safe = kmalloc(sizeof(*dma_safe), GFP_KERNEL | GFP_DMA);

Please don't use GFP_DMA, kmalloc() should already return a DMA-safe
buffer (see [1]).

> + if (!dma_safe)
> + return ERR_PTR(-ENOMEM);
>  
>   addr_width = nor->addr_width;
>   read_dummy = nor->read_dummy;
> @@ -2890,7 +2895,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor 
> *nor, const u32 *smpt,
>   nor->read_opcode = SMPT_CMD_OPCODE(smpt[i]);
>   addr = smpt[i + 1];
>  
> - err = spi_nor_read_raw(nor, addr, 1, _byte);
> + err = spi_nor_read_raw(nor, addr, 1, dma_safe);
>   if (err) {
>   ret = ERR_PTR(err);
>   goto out;
> @@ -2900,7 +2905,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor 
> *nor, const u32 *smpt,
>* Build an index value that is used to select the Sector Map
>* Configuration that is currently in use.
>*/
> - map_id = map_id << 1 | !!(data_byte & read_data_mask);
> + map_id = map_id << 1 | !!(*dma_safe & read_data_mask);
>   ncmds++;
>   }
>  
> @@ -2941,6 +2946,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor 
> *nor, const u32 *smpt,
>  
>   /* fall through */
>  out:
> + kfree(dma_safe);
>   nor->addr_width = addr_width;
>   nor->read_dummy = read_dummy;
>   nor->read_opcode = read_opcode;

[1]https://elixir.bootlin.com/linux/latest/source/include/linux/gfp.h#L263


Re: [PATCH 3/7] mtd: spi-nor: add restriction for nmaps in smpt parser

2018-11-08 Thread Boris Brezillon
On Thu, 8 Nov 2018 11:07:11 +
 wrote:

> The map selector is limited to a maximum of 8 bits, allowing
> for a maximum of 256 possible map configurations. The total
> number of map configurations should be addressable by the
> total number of bits described by the detection commands.
> 
> For example: if there are five to eight possible sector map
> configurations, at least three configuration detection commands
> will be needed to extract three bits of configuration selection
> information from the device in order to identify which configuration
> is currently in use.
> 
> Suggested-by: Boris Brezillon 

I don't remember suggesting this :-).

> Signed-off-by: Tudor Ambarus 
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 59dcedb08691..bd1866d714f2 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2868,7 +2868,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor 
> *nor, const u32 *smpt,
>   const u32 *ret = NULL;
>   u32 addr;
>   int err;
> - u8 i;
> + u8 i, ncmds, nmaps;
>   u8 addr_width, read_opcode, read_dummy;
>   u8 read_data_mask, data_byte, map_id;
>  
> @@ -2877,6 +2877,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor 
> *nor, const u32 *smpt,
>   read_opcode = nor->read_opcode;
>  
>   map_id = 0;
> + ncmds = 0;
>   /* Determine if there are any optional Detection Command Descriptors */
>   for (i = 0; i < smpt_len; i += 2) {
>   if (smpt[i] & SMPT_DESC_TYPE_MAP)
> @@ -2896,6 +2897,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor 
> *nor, const u32 *smpt,
>* Configuration that is currently in use.
>*/
>   map_id = map_id << 1 | !!(data_byte & read_data_mask);
> + ncmds++;
>   }
>  
>   /*
> @@ -2905,7 +2907,16 @@ static const u32 *spi_nor_get_map_in_use(struct 
> spi_nor *nor, const u32 *smpt,
>*
>* Find the matching configuration map.
>*/
> - while (i < smpt_len) {
> + for (nmaps = 0; i < smpt_len; nmaps++) {
> + /*
> +  * The map selector is limited to a maximum of 8 bits, allowing
> +  * for a maximum of 256 possible map configurations. The total
> +  * number of map configurations should be addressable by the
> +  * total number of bits described by the detection commands.
> +  */
> + if (ncmds && nmaps >= (1 << (ncmds + 1)))
> + break;
> +

Maybe I missed something but it sounds like this change is just
optimizing the SPMT parsing a bit, and to be honest, I'm not sure this
is really needed. Most of the time, smpt_len will be rather small, so
trying to bail out earlier is not bringing much perf improvements.

>   if (SMPT_MAP_ID(smpt[i]) == map_id) {
>   ret = smpt + i;
>   break;



Re: [PATCH 2/7] mtd: spi-nor: fix iteration over smpt array

2018-11-08 Thread Boris Brezillon
On Thu, 8 Nov 2018 11:07:09 +
 wrote:

> Iterate over smpt array using its starting address and length
> instead of the blindly iterations that used data found in the array.

 ^blind

> 
> This prevents possible memory accesses outside of the smpt array
> boundaries in case software, or manufacturers, misrepresent smpt
> array fields.
> 
> Suggested-by: Boris Brezillon 
> Signed-off-by: Tudor Ambarus 

I think we should consider this patch as a fix. Would you mind adding a
Fixes tag?

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 39 +--
>  1 file changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 2cdf96013689..59dcedb08691 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2860,12 +2860,15 @@ static u8 spi_nor_smpt_read_dummy(const struct 
> spi_nor *nor, const u32 settings)
>   * spi_nor_get_map_in_use() - get the configuration map in use
>   * @nor: pointer to a 'struct spi_nor'
>   * @smpt:pointer to the sector map parameter table
> + * @smpt_len:sector map parameter table length
>   */
> -static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 
> *smpt)
> +static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 
> *smpt,
> +  u8 smpt_len)
>  {
>   const u32 *ret = NULL;
> - u32 i, addr;
> + u32 addr;
>   int err;
> + u8 i;
>   u8 addr_width, read_opcode, read_dummy;
>   u8 read_data_mask, data_byte, map_id;
>  
> @@ -2874,9 +2877,10 @@ static const u32 *spi_nor_get_map_in_use(struct 
> spi_nor *nor, const u32 *smpt)
>   read_opcode = nor->read_opcode;
>  
>   map_id = 0;
> - i = 0;
>   /* Determine if there are any optional Detection Command Descriptors */
> - while (!(smpt[i] & SMPT_DESC_TYPE_MAP)) {
> + for (i = 0; i < smpt_len; i += 2) {
> + if (smpt[i] & SMPT_DESC_TYPE_MAP)
> + break;

nit: add a blank line here.

>   read_data_mask = SMPT_CMD_READ_DATA(smpt[i]);
>   nor->addr_width = spi_nor_smpt_addr_width(nor, smpt[i]);
>   nor->read_dummy = spi_nor_smpt_read_dummy(nor, smpt[i]);
> @@ -2892,18 +2896,33 @@ static const u32 *spi_nor_get_map_in_use(struct 
> spi_nor *nor, const u32 *smpt)
>* Configuration that is currently in use.
>*/
>   map_id = map_id << 1 | !!(data_byte & read_data_mask);
> - i = i + 2;
>   }
>  
> - /* Find the matching configuration map */
> - while (SMPT_MAP_ID(smpt[i]) != map_id) {
> + /*
> +  * If command descriptors are provided, they always precede map
> +  * descriptors in the table. There is no need to start the iteration
> +  * over smpt array all over again.
> +  *
> +  * Find the matching configuration map.
> +  */
> + while (i < smpt_len) {
> + if (SMPT_MAP_ID(smpt[i]) == map_id) {
> + ret = smpt + i;
> + break;
> + }
> +
> + /*
> +  * If there are no more configuration map descriptors and no
> +  * configuration ID matched the configuration identifier, the
> +  * sector address map is unknown.
> +  */
>   if (smpt[i] & SMPT_DESC_END)
> - goto out;
> + break;
> +
>   /* increment the table index to the next map */
>   i += SMPT_MAP_REGION_COUNT(smpt[i]) + 1;
>   }
>  
> - ret = smpt + i;
>   /* fall through */
>  out:
>   nor->addr_width = addr_width;
> @@ -3025,7 +3044,7 @@ static int spi_nor_parse_smpt(struct spi_nor *nor,
>   for (i = 0; i < smpt_header->length; i++)
>   smpt[i] = le32_to_cpu(smpt[i]);
>  
> - sector_map = spi_nor_get_map_in_use(nor, smpt);
> + sector_map = spi_nor_get_map_in_use(nor, smpt, smpt_header->length);
>   if (!sector_map) {
>   ret = -EINVAL;
>   goto out;



[GIT PULL] mtd: Fixes for 4.20-rc2

2018-11-08 Thread Boris Brezillon
Hello Linus,

Here is the MTD fixes PR for 4.20-rc2.

Regards,

Boris

The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a:

  Linux 4.20-rc1 (2018-11-04 15:37:52 -0800)

are available in the Git repository at:

  git://git.infradead.org/linux-mtd.git tags/mtd/fixes-for-4.20-rc2

for you to fetch changes up to 98ee3fc7ef8395f8b7a379e6608aee91efc66d48:

  mtd: nand: Fix nanddev_pos_next_page() kernel-doc header (2018-11-06 17:40:31 
+0100)


MTD changes:
* Kill a VLA in sa1100

SPI NOR changes:
* Make sure ->addr_width is restored when SFDP parsing fails
* Propate errors happening in cqspi_direct_read_execute()

NAND changes:
* Fix kernel-doc mismatch
* Fix nanddev_neraseblocks() to return the correct value
* Avoid selection of BCH_CONST_PARAMS when some users require
  dynamic BCH settings


Arnd Bergmann (1):
  mtd: docg3: don't set conflicting BCH_CONST_PARAMS option

Boris Brezillon (4):
  mtd: nand: Fix nanddev_neraseblocks()
  mtd: spi-nor: Reset nor->addr_width when SFDP parsing failed
  mtd: sa1100: avoid VLA in sa1100_setup_mtd
  mtd: nand: Fix nanddev_pos_next_page() kernel-doc header

Christophe JAILLET (1):
  mtd: spi-nor: cadence-quadspi: Return error code in 
cqspi_direct_read_execute()

Randy Dunlap (1):
  mtd: nand: drop kernel-doc notation for a deleted function parameter

 drivers/mtd/devices/Kconfig   |  2 +-
 drivers/mtd/maps/sa1100-flash.c   | 10 +-
 drivers/mtd/nand/raw/nand_base.c  |  1 -
 drivers/mtd/spi-nor/cadence-quadspi.c |  2 +-
 drivers/mtd/spi-nor/spi-nor.c |  6 --
 include/linux/mtd/nand.h  |  7 +++
 6 files changed, 18 insertions(+), 10 deletions(-)


Re: [PATCH][i3c-next] i3c: fix incorrect update to max_read_ds

2018-11-08 Thread Boris Brezillon
On Wed,  7 Nov 2018 20:42:08 +
Hi Colin,

Colin King  wrote:

> From: Colin Ian King 
> 
> Currently max_read_ds is being updated twice, which is incorrect.
> The second assignment should be to max_write_ds instead. Fix this.
> 
> Detected by CoverityScan, CID#1475379 ("Unused value")
> 
> Fixes: 50cacdabeae1 ("i3c: Add core I3C infrastructure")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/i3c/master.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 68d6553dbd75..0ea7bb045fad 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1093,7 +1093,7 @@ static int i3c_master_getmxds_locked(struct 
> i3c_master_controller *master,
>   }
>  
>   info->max_read_ds = getmaxds->maxrd;
> - info->max_read_ds = getmaxds->maxwr;
> + info->max_write_ds = getmaxds->maxwr;

Thanks for the fix.

Since the I3C driver is not merged yet, would you mind if I squash your
changes in the commit introducing the bug?

Regards,

Boris


Re: [PATCH] mtd: nand: spi: Add initial support for Toshiba TC58CVG2S0H

2018-11-08 Thread Boris Brezillon
Nitpick: subject prefix should be "mtd: spinand" not
"mtd: nand: spi" :-).

On Thu, 8 Nov 2018 08:32:11 +
Schrempf Frieder  wrote:

> Add minimal support for the Toshiba TC58CVG2S0H SPI NAND chip.
> 
> Signed-off-by: Frieder Schrempf 
> ---
>  drivers/mtd/nand/spi/Makefile  |   2 +-
>  drivers/mtd/nand/spi/core.c|   1 +
>  drivers/mtd/nand/spi/toshiba.c | 136 
>  include/linux/mtd/spinand.h|   1 +
>  4 files changed, 139 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> index b74e074..be5f735 100644
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1,3 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0
> -spinand-objs := core.o macronix.o micron.o winbond.o
> +spinand-objs := core.o macronix.o micron.o toshiba.o winbond.o
>  obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 30f8364..87bdf2a 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -766,6 +766,7 @@ static const struct nand_ops spinand_ops = {
>  static const struct spinand_manufacturer *spinand_manufacturers[] = {
>   _spinand_manufacturer,
>   _spinand_manufacturer,
> + _spinand_manufacturer,
>   _spinand_manufacturer,
>  };
>  
> diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
> new file mode 100644
> index 000..294bcf6
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/toshiba.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 exceet electronics GmbH
> + * Copyright (c) 2018 Kontron Electronics GmbH
> + *
> + * Author: Frieder Schrempf 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#define SPINAND_MFR_TOSHIBA  0x98
> +
> +static SPINAND_OP_VARIANTS(read_cache_variants,
> + SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(write_cache_variants,
> + SPINAND_PROG_LOAD(true, 0, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(update_cache_variants,
> + SPINAND_PROG_LOAD(false, 0, NULL, 0));
> +
> +static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
> +  struct mtd_oob_region *region)
> +{
> + if (section > 7)
> + return -ERANGE;
> +
> + region->offset = 128 + 16 * section;
> + region->length = 16;
> +
> +
> + return 0;
> +}
> +
> +static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
> +   struct mtd_oob_region *region)
> +{
> + if (section > 7)
> + return -ERANGE;
> +
> + region->offset = 2 + 16 * section;
> + region->length = 14;
> +
> +
> + return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops tc58cvg2s0h_ooblayout = {
> + .ecc = tc58cvg2s0h_ooblayout_ecc,
> + .free = tc58cvg2s0h_ooblayout_free,
> +};
> +
> +static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
> +   u8 status)
> +{
> + struct nand_device *nand = spinand_to_nand(spinand);
> + u8 mbf = 0;
> + struct spi_mem_op op = SPINAND_GET_FEATURE_OP(0x30, );
> +
> + switch (status & STATUS_ECC_MASK) {
> + case STATUS_ECC_NO_BITFLIPS:
> + return 0;
> +
> + case STATUS_ECC_UNCOR_ERROR:
> + return -EBADMSG;
> +
> + case STATUS_ECC_HAS_BITFLIPS:
> + /*
> +  * Let's try to retrieve the real maximum number of bitflips
> +  * in order to avoid forcing the wear-leveling layer to move
> +  * data around if it's not necessary.
> +  */
> + if (spi_mem_exec_op(spinand->spimem, ))
> + return nand->eccreq.strength;
> +
> + mbf >>= 4;
> +
> + if (WARN_ON(mbf > nand->eccreq.strength || !mbf))
> + return nand->eccreq.strength;
> +
> + return mbf;
> +
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct spinand_info toshiba_spinand_table[] = {
> + SPINAND_INFO("TC58CVG2S0H", 0xCD,
> +  NAND_MEMORG(1, 4096, 256, 64, 2048, 1, 1, 1),
> +  NAND_ECCREQ(8, 512),
> +  SPINAND_INFO_OP_VARIANTS(_cache_variants,
> +   _cache_variants,
> +   _cache_variants),
> +  SPINAND_HAS_QE_BIT,
> +  SPINAND_ECCINFO(_ooblayout,
> +  tc58cvg2s0h_ecc_get_status)),
> +};
> +
> +static int toshiba_spinand_detect(struct spinand_device *spinand)
> +{
> + u8 *id = 

Re: Applied "spi: Add QuadSPI driver for Atmel SAMA5D2" to the spi tree

2018-11-08 Thread Boris Brezillon
Hi Mark,

On Wed, 7 Nov 2018 15:18:36 +
Mark Brown  wrote:

> On Wed, Nov 07, 2018 at 03:03:27PM +, Mark Brown wrote:
> > The patch
> > 
> >spi: Add QuadSPI driver for Atmel SAMA5D2
> > 
> > has been applied to the spi tree at
> > 
> >https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git   
> 
> Sorry, I got confused about which patch series Boris had asked me to
> apply for him - it wasn't this one!  Since the review looked like it was
> just one minor issue last time I'll just leave them for now, they were
> applied on a separate branch for sharing with the MTD tree so it's easy
> for me to drop them if that's required or if I should put together the
> pull request for MTD.  Sorry for any confusion this ends up causing,
> it's entirely my mistake.

I'm probably missing something, but I don't see where you made a
mistake :-). It's indeed the series I asked you to merge, the only
comment I had was actually a suggestion to help Piotr solve his perf
issue (which was already present in the old driver).

And thanks for creating the mtd topic branch BTW.

Regards,

Boris


Re: [PATCH v4 03/10] dt-bindings: spi: Adjust the bindings for the FSL QSPI driver

2018-11-08 Thread Boris Brezillon
On Wed,  7 Nov 2018 15:43:20 +0100
Frieder Schrempf  wrote:

> From: Frieder Schrempf 
> 
> Adjust the documentation of the new SPI memory interface based
> driver to reflect the new drivers settings.
> 
> The "old" driver was using the "fsl,qspi-has-second-chip" property to
> select one of two dual chip setups (two chips on one bus or two chips
> on separate buses). And it used the order in which the subnodes are
> defined in the dt to select the CS, the chip is connected to.
> 
> Both methods are wrong and in fact the "reg" property should be used to
> determine which bus and CS a chip is connected to. This also enables us
> to use different setups than just single chip, or symmetric dual chip.
> 
> So the porting of the driver from the MTD to the SPI framework actually
> enforces the use of the "reg" properties and makes
> "fsl,qspi-has-second-chip" superfluous.
> 
> As all boards that have "fsl,qspi-has-second-chip" set, also have
> correct "reg" properties, the removal of this property shouldn't lead to
> any incompatibilities.
> 
> The only compatibility issues I can see are with imx6sx-sdb.dts and
> imx6sx-sdb-reva.dts, which have their reg properties set incorrectly
> (see explanation here: [2]), all other boards should stay compatible.
> 
> Also the "big-endian" flag was removed, as this setting is now selected
> by the driver, depending on which SoC is in use.
> 
> Signed-off-by: Frieder Schrempf 
> ---
>  .../devicetree/bindings/spi/spi-fsl-qspi.txt| 21 +---
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt 
> b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
> index 483e9cf..6d7c9ec 100644
> --- a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
> @@ -3,9 +3,8 @@
>  Required properties:
>- compatible : Should be "fsl,vf610-qspi", "fsl,imx6sx-qspi",
>"fsl,imx7d-qspi", "fsl,imx6ul-qspi",
> -  "fsl,ls1021a-qspi"
> +  "fsl,ls1021a-qspi", "fsl,ls2080a-qspi"
>or
> -  "fsl,ls2080a-qspi" followed by "fsl,ls1021a-qspi",

Looks like this change is not related to this commit, and I'm not sure
it's even needed. Plus, the order differs from the previous
description, so, if the doc was right before this change it should be:

"fsl,ls2080a-qspi", "fsl,ls1021a-qspi"

>"fsl,ls1043a-qspi" followed by "fsl,ls1021a-qspi"
>- reg : the first contains the register location and length,
>the second contains the memory mapping address and length
> @@ -14,15 +13,13 @@ Required properties:
>- clocks : The clocks needed by the QuadSPI controller
>- clock-names : Should contain the name of the clocks: "qspi_en" and 
> "qspi".
>  
> -Optional properties:
> -  - fsl,qspi-has-second-chip: The controller has two buses, bus A and bus B.
> -  Each bus can be connected with two NOR flashes.
> -   Most of the time, each bus only has one NOR flash
> -   connected, this is the default case.
> -   But if there are two NOR flashes connected to the
> -   bus, you should enable this property.
> -   (Please check the board's schematic.)
> -  - big-endian : That means the IP register is big endian
> +Required SPI slave node properties:
> +  - reg: There are two buses (A and B) with two chip selects each.
> +  This encodes to which bus and CS the flash is connected:
> + <0>: Bus A, CS 0
> + <1>: Bus A, CS 1
> + <2>: Bus B, CS 0
> + <3>: Bus B, CS 1
>  
>  Example:
>  
> @@ -40,7 +37,7 @@ qspi0: quadspi@40044000 {
>   };
>  };
>  
> -Example showing the usage of two SPI NOR devices:
> +Example showing the usage of two SPI NOR devices on bus A:
>  
>   {
>   pinctrl-names = "default";



  1   2   3   4   5   6   7   8   9   10   >