[PATCH 0/2] Untangle Spansion S25F{L|S}512S chip IDs

[PATCH 0/2] Untangle Spansion S25F{L|S}512S chip IDs

[Sergei Shtylyov]

Hello!

Here's the set of 2 patches against the 'spi-nor/next' branch of 'linux-mtd.git'
repo. We're untangling the S25FS512S being taken for S25FL512S by the SPI NOR code
due to omitting the family ID byte in the latter chip's ID. I'm proposing to use
the full 6-byte JEDEC ID for both these chips.

[1/2] mtd: spi-nor: add Spansion S25FS512S ID
[2/2] mtd: spi-nor: refine Spansion S25FL512S ID

MBR, Sergei

[PATCH 1/2] mtd: spi-nor: add Spansion S25FS512S ID

[Sergei Shtylyov]

Spansion S25FS512S flash is currently misdetected as S25FL512S since the
latter uses 5-byte JEDEC ID, while the 6th ID byte (family ID) is different
on those chips. Add the 6-byte S25FS512S ID before S25FL512S ID in order
not to break the existing S25FS512S users.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 drivers/mtd/spi-nor/spi-nor.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux-mtd/drivers/mtd/spi-nor/spi-nor.c
===================================================================
--- linux-mtd.orig/drivers/mtd/spi-nor/spi-nor.c
+++ linux-mtd/drivers/mtd/spi-nor/spi-nor.c
@@ -1887,6 +1887,7 @@ static const struct flash_info spi_nor_i
 	{ "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
 	{ "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
+	{ "s25fs512s",  INFO6(0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
 	{ "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
 	{ "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
 	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },

Re: [PATCH 1/2] mtd: spi-nor: add Spansion S25FS512S ID

[Tudor.Ambarus]

On 01/16/2019 07:51 PM, Sergei Shtylyov wrote:
> Spansion S25FS512S flash is currently misdetected as S25FL512S since the
> latter uses 5-byte JEDEC ID, while the 6th ID byte (family ID) is different
> on those chips. Add the 6-byte S25FS512S ID before S25FL512S ID in order
> not to break the existing S25FS512S users.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

> 
> ---
>  drivers/mtd/spi-nor/spi-nor.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux-mtd/drivers/mtd/spi-nor/spi-nor.c
> ===================================================================
> --- linux-mtd.orig/drivers/mtd/spi-nor/spi-nor.c
> +++ linux-mtd/drivers/mtd/spi-nor/spi-nor.c
> @@ -1887,6 +1887,7 @@ static const struct flash_info spi_nor_i
>  	{ "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>  	{ "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
>  	{ "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> +	{ "s25fs512s",  INFO6(0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>  	{ "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>  	{ "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
>  	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [1/2] mtd: spi-nor: add Spansion S25FS512S ID

[Boris Brezillon]

On Wed, 2019-01-16 at 17:51:56 UTC, Sergei Shtylyov wrote:
> Spansion S25FS512S flash is currently misdetected as S25FL512S since the
> latter uses 5-byte JEDEC ID, while the 6th ID byte (family ID) is different
> on those chips. Add the 6-byte S25FS512S ID before S25FL512S ID in order
> not to break the existing S25FS512S users.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

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

Boris

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/2] mtd: spi-nor: refine Spansion S25FL512S ID

[Sergei Shtylyov]

Spansion S25FL512S ID is erroneously using 5-byte JEDEC ID, while the chip
family ID is stored in the 6th byte. Due to using only 5-byte ID, it's also
covering S25FS512S and now that we have added 6-byte ID for that chip, we
can convert S25FL512S to using a proper 6-byte ID as well...

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 drivers/mtd/spi-nor/spi-nor.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-mtd/drivers/mtd/spi-nor/spi-nor.c
===================================================================
--- linux-mtd.orig/drivers/mtd/spi-nor/spi-nor.c
+++ linux-mtd/drivers/mtd/spi-nor/spi-nor.c
@@ -1887,8 +1887,8 @@ static const struct flash_info spi_nor_i
 	{ "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
 	{ "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
+	{ "s25fl512s",  INFO6(0x010220, 0x4d0080, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
 	{ "s25fs512s",  INFO6(0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
-	{ "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
 	{ "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
 	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
 	{ "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },

Re: [PATCH 2/2] mtd: spi-nor: refine Spansion S25FL512S ID

[Tudor.Ambarus]

On 01/16/2019 07:53 PM, Sergei Shtylyov wrote:
> Spansion S25FL512S ID is erroneously using 5-byte JEDEC ID, while the chip
> family ID is stored in the 6th byte. Due to using only 5-byte ID, it's also
> covering S25FS512S and now that we have added 6-byte ID for that chip, we
> can convert S25FL512S to using a proper 6-byte ID as well...
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

> 
> ---
>  drivers/mtd/spi-nor/spi-nor.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-mtd/drivers/mtd/spi-nor/spi-nor.c
> ===================================================================
> --- linux-mtd.orig/drivers/mtd/spi-nor/spi-nor.c
> +++ linux-mtd/drivers/mtd/spi-nor/spi-nor.c
> @@ -1887,8 +1887,8 @@ static const struct flash_info spi_nor_i
>  	{ "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>  	{ "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
>  	{ "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> +	{ "s25fl512s",  INFO6(0x010220, 0x4d0080, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>  	{ "s25fs512s",  INFO6(0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> -	{ "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>  	{ "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
>  	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
>  	{ "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },
> 
> 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [2/2] mtd: spi-nor: refine Spansion S25FL512S ID

[Boris Brezillon]

On Wed, 2019-01-16 at 17:53:25 UTC, Sergei Shtylyov wrote:
> Spansion S25FL512S ID is erroneously using 5-byte JEDEC ID, while the chip
> family ID is stored in the 6th byte. Due to using only 5-byte ID, it's also
> covering S25FS512S and now that we have added 6-byte ID for that chip, we
> can convert S25FL512S to using a proper 6-byte ID as well...
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

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

Boris

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: spi-nor: refine Spansion S25FL512S ID

[Geert Uytterhoeven]

Hi Sergei,

On Wed, Jan 16, 2019 at 6:53 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Spansion S25FL512S ID is erroneously using 5-byte JEDEC ID, while the chip
> family ID is stored in the 6th byte. Due to using only 5-byte ID, it's also
> covering S25FS512S and now that we have added 6-byte ID for that chip, we
> can convert S25FL512S to using a proper 6-byte ID as well...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

This is now commit a2126b0a010905e5 ("mtd: spi-nor: refine Spansion
S25FL512S ID"), and turns out to cause a regression on r8a7791/koelsch.
Dmesg diff before/after:

    -m25p80 spi0.0: s25fl512s (65536 Kbytes)
    -3 fixed-partitions partitions found on MTD device spi0.0
    -Creating 3 MTD partitions on "spi0.0":
    -0x000000000000-0x000000080000 : "loader"
    -0x000000080000-0x000000600000 : "user"
    -0x000000600000-0x000004000000 : "flash"
    +m25p80 spi0.0: unrecognized JEDEC id bytes: 01, 02, 20

As the (old) U-Boot on my Koelsch keeps many module clocks enabled, I
typically merge in my topic/renesas-debug branch, which makes sure all
non-critical module clocks are disabled early during boot, to catch
drivers not properly implementing Runtime PM.

However, it turns out this has some impact on JEDEC ID detection:
  - When module clocks are left untouched, spi_nor_read_id() reads
    0x01:0x02:0x20:0x4d:0x00:0x80.
  - When my debug code has disabled module clocks during early boot,
    The last byte is 0x00.

Before the above commit, only the first 5 bytes were compared, and the
last byte was ignored, thus not causing problems.
When comparing all 6 bytes, detection fails if the last byte is 0x00.

I believe mainline U-Boot for R-Car Gen2 boards doesn't keep the QSPI
module clock enabled, so this commit may breaks those boards.

To be investigated more (e.g. with a logic analyzer)...

> --- linux-mtd.orig/drivers/mtd/spi-nor/spi-nor.c
> +++ linux-mtd/drivers/mtd/spi-nor/spi-nor.c
> @@ -1887,8 +1887,8 @@ static const struct flash_info spi_nor_i
>         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
>         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> +       { "s25fl512s",  INFO6(0x010220, 0x4d0080, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>         { "s25fs512s",  INFO6(0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> -       { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>         { "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
>         { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
>         { "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: spi-nor: refine Spansion S25FL512S ID

[Tudor.Ambarus]

Hi, Geert,

On 03/05/2019 04:05 PM, Geert Uytterhoeven wrote:
> Hi Sergei,
> 
> On Wed, Jan 16, 2019 at 6:53 PM Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
>> Spansion S25FL512S ID is erroneously using 5-byte JEDEC ID, while the chip
>> family ID is stored in the 6th byte. Due to using only 5-byte ID, it's also
>> covering S25FS512S and now that we have added 6-byte ID for that chip, we
>> can convert S25FL512S to using a proper 6-byte ID as well...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> This is now commit a2126b0a010905e5 ("mtd: spi-nor: refine Spansion
> S25FL512S ID"), and turns out to cause a regression on r8a7791/koelsch.
> Dmesg diff before/after:
> 
>     -m25p80 spi0.0: s25fl512s (65536 Kbytes)
>     -3 fixed-partitions partitions found on MTD device spi0.0
>     -Creating 3 MTD partitions on "spi0.0":
>     -0x000000000000-0x000000080000 : "loader"
>     -0x000000080000-0x000000600000 : "user"
>     -0x000000600000-0x000004000000 : "flash"
>     +m25p80 spi0.0: unrecognized JEDEC id bytes: 01, 02, 20
> 
> As the (old) U-Boot on my Koelsch keeps many module clocks enabled, I
> typically merge in my topic/renesas-debug branch, which makes sure all
> non-critical module clocks are disabled early during boot, to catch
> drivers not properly implementing Runtime PM.
> 
> However, it turns out this has some impact on JEDEC ID detection:
>   - When module clocks are left untouched, spi_nor_read_id() reads
>     0x01:0x02:0x20:0x4d:0x00:0x80.
>   - When my debug code has disabled module clocks during early boot,
>     The last byte is 0x00.
> 
> Before the above commit, only the first 5 bytes were compared, and the
> last byte was ignored, thus not causing problems.
> When comparing all 6 bytes, detection fails if the last byte is 0x00.
> 

We'll have to understand why just the 6th byte is different.

> I believe mainline U-Boot for R-Car Gen2 boards doesn't keep the QSPI
> module clock enabled, so this commit may breaks those boards.

Who provides the flash's input clock and at what frequency?

Cheers,
ta

> 
> To be investigated more (e.g. with a logic analyzer)...
> 
>> --- linux-mtd.orig/drivers/mtd/spi-nor/spi-nor.c
>> +++ linux-mtd/drivers/mtd/spi-nor/spi-nor.c
>> @@ -1887,8 +1887,8 @@ static const struct flash_info spi_nor_i
>>         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>>         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
>>         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>> +       { "s25fl512s",  INFO6(0x010220, 0x4d0080, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>>         { "s25fs512s",  INFO6(0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>> -       { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>>         { "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
>>         { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
>>         { "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: spi-nor: refine Spansion S25FL512S ID

[Tudor.Ambarus]

On 03/05/2019 04:05 PM, Geert Uytterhoeven wrote:
>     +m25p80 spi0.0: unrecognized JEDEC id bytes: 01, 02, 20

side note: first 3 id bytes are correct, we should print all id_len bytes on error.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: spi-nor: refine Spansion S25FL512S ID

[Geert Uytterhoeven]

On Tue, Mar 5, 2019 at 3:05 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Wed, Jan 16, 2019 at 6:53 PM Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
> > Spansion S25FL512S ID is erroneously using 5-byte JEDEC ID, while the chip
> > family ID is stored in the 6th byte. Due to using only 5-byte ID, it's also
> > covering S25FS512S and now that we have added 6-byte ID for that chip, we
> > can convert S25FL512S to using a proper 6-byte ID as well...
> >
> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> This is now commit a2126b0a010905e5 ("mtd: spi-nor: refine Spansion
> S25FL512S ID"), and turns out to cause a regression on r8a7791/koelsch.
> Dmesg diff before/after:
>
>     -m25p80 spi0.0: s25fl512s (65536 Kbytes)
>     -3 fixed-partitions partitions found on MTD device spi0.0
>     -Creating 3 MTD partitions on "spi0.0":
>     -0x000000000000-0x000000080000 : "loader"
>     -0x000000080000-0x000000600000 : "user"
>     -0x000000600000-0x000004000000 : "flash"
>     +m25p80 spi0.0: unrecognized JEDEC id bytes: 01, 02, 20
>
> As the (old) U-Boot on my Koelsch keeps many module clocks enabled, I
> typically merge in my topic/renesas-debug branch, which makes sure all
> non-critical module clocks are disabled early during boot, to catch
> drivers not properly implementing Runtime PM.
>
> However, it turns out this has some impact on JEDEC ID detection:
>   - When module clocks are left untouched, spi_nor_read_id() reads
>     0x01:0x02:0x20:0x4d:0x00:0x80.
>   - When my debug code has disabled module clocks during early boot,
>     The last byte is 0x00.
>
> Before the above commit, only the first 5 bytes were compared, and the
> last byte was ignored, thus not causing problems.
> When comparing all 6 bytes, detection fails if the last byte is 0x00.
>
> I believe mainline U-Boot for R-Car Gen2 boards doesn't keep the QSPI
> module clock enabled, so this commit may breaks those boards.
>
> To be investigated more (e.g. with a logic analyzer)...

The FLASH returns the correct data, but it is not received correctly.
When reading 16 bytes, thus including the ASCII model at offset 6/7:

  - actual:   01 02 20 4d 00 80 47 31 82 ff ff ff ff ff ff ff
  - received: 01 02 20 4d 00 00 8e 63 05 ff ff ff ff ff ff fe

When retrying the operation, data is received correctly.

This happens due to a Runtime PM related bug in the initialization code
of the spi-rspi driver: if the module clock is not running (disabled by
my debug code, the boot loader, or the clk_disable_unused late
initcall), the SPI controller is not initialized properly.  The first
transfer still manages to read some correct data, which used to be
sufficient to identify the S25FL512S FLASH chip, before the sixth byte
was also considered.

Will send a quick fix, and a proper solution requiring more refactoring
later.

Sorry for the noise...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/2] Untangle Spansion S25F{L|S}512S chip IDs

[Tudor.Ambarus]

Hi, Sergei,

On 01/16/2019 07:49 PM, Sergei Shtylyov wrote:
> Hello!
> 
> Here's the set of 2 patches against the 'spi-nor/next' branch of 'linux-mtd.git'
> repo. We're untangling the S25FS512S being taken for S25FL512S by the SPI NOR code
> due to omitting the family ID byte in the latter chip's ID. I'm proposing to use
> the full 6-byte JEDEC ID for both these chips.

The patch set is looking good. If these patches are based on Cyrille's
suggestion from [1], we can give him credit by adding:
Suggested-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
We'll need his approval if that's the case.

If you're going to resubmit, it's a good time to get rid of the 80 char limit
checkpatch warning. I would recommend this way of formatting entries, because it
has fewer lines:

        { "mx25u12835f", INFO(0xc22538, 0, 64 * 1024, 256,
                         SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

Of course, if this is not the case, and you came with this proposal without
knowing about Cyrille's suggestion, I'm not going to ask you to resubmit just
for this little cosmetic change, just say.

Thanks,
ta

[1] https://patchwork.kernel.org/patch/10595835/
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/2] Untangle Spansion S25F{L|S}512S chip IDs

[Sergei Shtylyov]

Hello!

On 01/21/2019 11:28 AM, Tudor.Ambarus@microchip.com wrote:

>> Here's the set of 2 patches against the 'spi-nor/next' branch of 'linux-mtd.git'
>> repo. We're untangling the S25FS512S being taken for S25FL512S by the SPI NOR code
>> due to omitting the family ID byte in the latter chip's ID. I'm proposing to use
>> the full 6-byte JEDEC ID for both these chips.
> 
> The patch set is looking good. If these patches are based on Cyrille's
> suggestion from [1], we can give him credit by adding:

   No, I even had trouble finding where Cyrille was suggesting that. :-)
BTW, looking at his message, I feel somewhat perplexed:

"Maybe you can do the magic by placing a new INFO6() entry for the S25FL512S,
just before the legacy INFO() entry for S25FL512S. Indeed entries are tested
in the order they appear inside the spi_nor_ids[] array from spi_nor_read_id(),
so the INFO6() entry would be tested 1st, trying to match 6 bytes, then the
INFO() entry only trying to match 3 bytes."

   Did he actually mean adding a new INFO6() entry for S25FS512S? Else that
sentence doesn't make a lot of sense to me. :-) Also INFO() entry matches 5 bytes,
not 3. 
   Anyway, I'm suggesting to get rid of the INFO() entry for S25FL512S altogether --
and that's not a part of his suggestion... perhaps it's too risky; I'm not sure
why 5-byte matching was used for S25FL512S in the 1st place.

> Suggested-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> We'll need his approval if that's the case.

[...]

> Of course, if this is not the case, and you came with this proposal without
> knowing about Cyrille's suggestion,

   Well, I remember you (or somebody else) pointing to that patchwork entry...
however, the idea I got was from the Renesas BSP patch.

> I'm not going to ask you to resubmit just
> for this little cosmetic change, just say.

   Thank you. :-)   

> Thanks,
> ta
> 
> [1] https://patchwork.kernel.org/patch/10595835/

MBR, Sergei

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4] mtd: devices: m25p80: Use the spi-mem dirmap API

[Sergei Shtylyov]

From: Boris Brezillon <boris.brezillon@bootlin.com>

Make use of the spi-mem direct mapping API to let advanced controllers
optimize read/write operations when they support direct mapping.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>

---
This is a leftover unmerged patch of the "spi: spi-mem: Add a direct mapping
API" series, atop of the 'mtd/next' branch of the MTD 'linux.git' repo.

Changes in v4:
- Add Yogesh's T-b and my S-o-b

Changes in v3:
- Make nor->read/write() functional before the direct mappings have been
  created
- Add Miquel's R-b

Changes in v2:
- Rename the dirmap fields
- Return directly after calling dirmap_read/write() and let the spi-nor
  framework call us again if those functions returned less than the
  requested length

 drivers/mtd/devices/m25p80.c |  102 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 99 insertions(+), 3 deletions(-)

Index: linux/drivers/mtd/devices/m25p80.c
===================================================================
--- linux.orig/drivers/mtd/devices/m25p80.c
+++ linux/drivers/mtd/devices/m25p80.c
@@ -31,8 +31,70 @@
 struct m25p {
 	struct spi_mem		*spimem;
 	struct spi_nor		spi_nor;
+	struct {
+		struct spi_mem_dirmap_desc *rdesc;
+		struct spi_mem_dirmap_desc *wdesc;
+	} dirmap;
 };
 
+static int m25p_create_write_dirmap(struct m25p *flash)
+{
+	struct spi_nor *nor = &flash->spi_nor;
+	struct spi_mem_dirmap_info info = {
+		.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
+				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 1),
+				      SPI_MEM_OP_NO_DUMMY,
+				      SPI_MEM_OP_DATA_OUT(0, NULL, 1)),
+		.offset = 0,
+		.length = flash->spi_nor.mtd.size,
+	};
+	struct spi_mem_op *op = &info.op_tmpl;
+
+	/* get transfer protocols. */
+	op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
+	op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
+	op->dummy.buswidth = op->addr.buswidth;
+	op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
+
+	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
+		op->addr.nbytes = 0;
+
+	flash->dirmap.wdesc = spi_mem_dirmap_create(flash->spimem, &info);
+	if (IS_ERR(flash->dirmap.wdesc))
+		return PTR_ERR(flash->dirmap.wdesc);
+
+	return 0;
+}
+
+static int m25p_create_read_dirmap(struct m25p *flash)
+{
+	struct spi_nor *nor = &flash->spi_nor;
+	struct spi_mem_dirmap_info info = {
+		.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
+				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 1),
+				      SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
+				      SPI_MEM_OP_DATA_IN(0, NULL, 1)),
+		.offset = 0,
+		.length = flash->spi_nor.mtd.size,
+	};
+	struct spi_mem_op *op = &info.op_tmpl;
+
+	/* get transfer protocols. */
+	op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
+	op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
+	op->dummy.buswidth = op->addr.buswidth;
+	op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
+
+	/* convert the dummy cycles to the number of bytes */
+	op->dummy.nbytes = (nor->read_dummy * op->dummy.buswidth) / 8;
+
+	flash->dirmap.rdesc = spi_mem_dirmap_create(flash->spimem, &info);
+	if (IS_ERR(flash->dirmap.rdesc))
+		return PTR_ERR(flash->dirmap.rdesc);
+
+	return 0;
+}
+
 static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
 {
 	struct m25p *flash = nor->priv;
@@ -92,6 +154,9 @@ static ssize_t m25p80_write(struct spi_n
 				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
 	int ret;
 
+	if (flash->dirmap.wdesc)
+		return spi_mem_dirmap_write(flash->dirmap.wdesc, to, len, buf);
+
 	/* get transfer protocols. */
 	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
 	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
@@ -128,6 +193,9 @@ static ssize_t m25p80_read(struct spi_no
 	size_t remaining = len;
 	int ret;
 
+	if (flash->dirmap.rdesc)
+		return spi_mem_dirmap_read(flash->dirmap.rdesc, from, len, buf);
+
 	/* get transfer protocols. */
 	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
 	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
@@ -238,19 +306,47 @@ static int m25p_probe(struct spi_mem *sp
 	if (ret)
 		return ret;
 
-	return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
-				   data ? data->nr_parts : 0);
+	ret = m25p_create_write_dirmap(flash);
+	if (ret)
+		return ret;
+
+	ret = m25p_create_read_dirmap(flash);
+	if (ret)
+		goto err_destroy_write_dirmap;
+
+	ret = mtd_device_register(&nor->mtd, data ? data->parts : NULL,
+				  data ? data->nr_parts : 0);
+	if (ret)
+		goto err_destroy_read_dirmap;
+
+	return 0;
+
+err_destroy_read_dirmap:
+	spi_mem_dirmap_destroy(flash->dirmap.rdesc);
+
+err_destroy_write_dirmap:
+	spi_mem_dirmap_destroy(flash->dirmap.wdesc);
+
+	return ret;
 }
 
 
 static int m25p_remove(struct spi_mem *spimem)
 {
 	struct m25p	*flash = spi_mem_get_drvdata(spimem);
+	int ret;
 
 	spi_nor_restore(&flash->spi_nor);
 
 	/* Clean up MTD stuff. */
-	return mtd_device_unregister(&flash->spi_nor.mtd);
+	ret = mtd_device_unregister(&flash->spi_nor.mtd);
+	if (ret)
+		return ret;
+
+	spi_mem_dirmap_destroy(flash->dirmap.rdesc);
+	spi_mem_dirmap_destroy(flash->dirmap.wdesc);
+
+	return 0;
 }
 
 static void m25p_shutdown(struct spi_mem *spimem)

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4] mtd: devices: m25p80: Use the spi-mem dirmap API

[Vignesh Raghavendra]

Hi,

On 01-Jun-19 12:01 AM, Sergei Shtylyov wrote:
[...]
> 
>  drivers/mtd/devices/m25p80.c |  102 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 99 insertions(+), 3 deletions(-)
> 
> Index: linux/drivers/mtd/devices/m25p80.c
> ===================================================================
> --- linux.orig/drivers/mtd/devices/m25p80.c
> +++ linux/drivers/mtd/devices/m25p80.c
> @@ -31,8 +31,70 @@
>  struct m25p {
>  	struct spi_mem		*spimem;
>  	struct spi_nor		spi_nor;
> +	struct {
> +		struct spi_mem_dirmap_desc *rdesc;
> +		struct spi_mem_dirmap_desc *wdesc;
> +	} dirmap;
>  };
>  
> +static int m25p_create_write_dirmap(struct m25p *flash)
> +{
> +	struct spi_nor *nor = &flash->spi_nor;
> +	struct spi_mem_dirmap_info info = {
> +		.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
> +				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 1),
> +				      SPI_MEM_OP_NO_DUMMY,
> +				      SPI_MEM_OP_DATA_OUT(0, NULL, 1)),
> +		.offset = 0,
> +		.length = flash->spi_nor.mtd.size,
> +	};
> +	struct spi_mem_op *op = &info.op_tmpl;
> +
> +	/* get transfer protocols. */
> +	op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
> +	op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
> +	op->dummy.buswidth = op->addr.buswidth;
> +	op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
> +
> +	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
> +		op->addr.nbytes = 0;
> +
> +	flash->dirmap.wdesc = spi_mem_dirmap_create(flash->spimem, &info);

Could you change this and elsewhere to devm_spi_mem_dirmap_create() and
thus simplify error handling?

Regards
Vignesh


> +	if (IS_ERR(flash->dirmap.wdesc))
> +		return PTR_ERR(flash->dirmap.wdesc);
> +
> +	return 0;
> +}
> +
> +static int m25p_create_read_dirmap(struct m25p *flash)
> +{
> +	struct spi_nor *nor = &flash->spi_nor;
> +	struct spi_mem_dirmap_info info = {
> +		.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
> +				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 1),
> +				      SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
> +				      SPI_MEM_OP_DATA_IN(0, NULL, 1)),
> +		.offset = 0,
> +		.length = flash->spi_nor.mtd.size,
> +	};
> +	struct spi_mem_op *op = &info.op_tmpl;
> +
> +	/* get transfer protocols. */
> +	op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
> +	op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
> +	op->dummy.buswidth = op->addr.buswidth;
> +	op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
> +
> +	/* convert the dummy cycles to the number of bytes */
> +	op->dummy.nbytes = (nor->read_dummy * op->dummy.buswidth) / 8;
> +
> +	flash->dirmap.rdesc = spi_mem_dirmap_create(flash->spimem, &info);
> +	if (IS_ERR(flash->dirmap.rdesc))
> +		return PTR_ERR(flash->dirmap.rdesc);
> +
> +	return 0;
> +}
> +
>  static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
>  {
>  	struct m25p *flash = nor->priv;
> @@ -92,6 +154,9 @@ static ssize_t m25p80_write(struct spi_n
>  				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
>  	int ret;
>  
> +	if (flash->dirmap.wdesc)
> +		return spi_mem_dirmap_write(flash->dirmap.wdesc, to, len, buf);
> +
>  	/* get transfer protocols. */
>  	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
>  	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
> @@ -128,6 +193,9 @@ static ssize_t m25p80_read(struct spi_no
>  	size_t remaining = len;
>  	int ret;
>  
> +	if (flash->dirmap.rdesc)
> +		return spi_mem_dirmap_read(flash->dirmap.rdesc, from, len, buf);
> +
>  	/* get transfer protocols. */
>  	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
>  	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
> @@ -238,19 +306,47 @@ static int m25p_probe(struct spi_mem *sp
>  	if (ret)
>  		return ret;
>  
> -	return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
> -				   data ? data->nr_parts : 0);
> +	ret = m25p_create_write_dirmap(flash);
> +	if (ret)
> +		return ret;
> +
> +	ret = m25p_create_read_dirmap(flash);
> +	if (ret)
> +		goto err_destroy_write_dirmap;
> +
> +	ret = mtd_device_register(&nor->mtd, data ? data->parts : NULL,
> +				  data ? data->nr_parts : 0);
> +	if (ret)
> +		goto err_destroy_read_dirmap;
> +
> +	return 0;
> +
> +err_destroy_read_dirmap:
> +	spi_mem_dirmap_destroy(flash->dirmap.rdesc);
> +
> +err_destroy_write_dirmap:
> +	spi_mem_dirmap_destroy(flash->dirmap.wdesc);
> +
> +	return ret;
>  }
>  
>  
>  static int m25p_remove(struct spi_mem *spimem)
>  {
>  	struct m25p	*flash = spi_mem_get_drvdata(spimem);
> +	int ret;
>  
>  	spi_nor_restore(&flash->spi_nor);
>  
>  	/* Clean up MTD stuff. */
> -	return mtd_device_unregister(&flash->spi_nor.mtd);
> +	ret = mtd_device_unregister(&flash->spi_nor.mtd);
> +	if (ret)
> +		return ret;
> +
> +	spi_mem_dirmap_destroy(flash->dirmap.rdesc);
> +	spi_mem_dirmap_destroy(flash->dirmap.wdesc);
> +
> +	return 0;
>  }
>  
>  static void m25p_shutdown(struct spi_mem *spimem)
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH] mtd: chips: gen_probe: kill useless initializer in mtd_do_chip_probe()

[Sergei Shtylyov]

The 'mtd' local variable is initialized but this value is never used,
thus kill that initializer.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Thi patch is atop of the 'mtd/next' branch of the MTD 'linux.git' repo.

 drivers/mtd/chips/gen_probe.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/mtd/chips/gen_probe.c
===================================================================
--- linux.orig/drivers/mtd/chips/gen_probe.c
+++ linux/drivers/mtd/chips/gen_probe.c
@@ -20,7 +20,7 @@ static int genprobe_new_chip(struct map_
 
 struct mtd_info *mtd_do_chip_probe(struct map_info *map, struct chip_probe *cp)
 {
-	struct mtd_info *mtd = NULL;
+	struct mtd_info *mtd;
 	struct cfi_private *cfi;
 
 	/* First probe the map to see if we have CFI stuff there. */

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: chips: gen_probe: kill useless initializer in mtd_do_chip_probe()

[Vignesh Raghavendra]

On 04/07/19 1:56 AM, Sergei Shtylyov wrote:
> The 'mtd' local variable is initialized but this value is never used,
> thus kill that initializer.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> Thi patch is atop of the 'mtd/next' branch of the MTD 'linux.git' repo.

Merged into git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next

Thanks!

> 
>  drivers/mtd/chips/gen_probe.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux/drivers/mtd/chips/gen_probe.c
> ===================================================================
> --- linux.orig/drivers/mtd/chips/gen_probe.c
> +++ linux/drivers/mtd/chips/gen_probe.c
> @@ -20,7 +20,7 @@ static int genprobe_new_chip(struct map_
>  
>  struct mtd_info *mtd_do_chip_probe(struct map_info *map, struct chip_probe *cp)
>  {
> -	struct mtd_info *mtd = NULL;
> +	struct mtd_info *mtd;
>  	struct cfi_private *cfi;
>  
>  	/* First probe the map to see if we have CFI stuff there. */
> 

-- 
Regards
Vignesh

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH] mtd: cfi_util: use DIV_ROUND_UP() in cfi_udelay()

[Sergei Shtylyov]

cfi_udelay() open-codes DIV_ROUND_UP(), violating the kernel coding style
while  doing this...

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
This patch is atop of the 'mtd/next' branch of the MTD 'linux.git' repo.

 drivers/mtd/chips/cfi_util.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/mtd/chips/cfi_util.c
===================================================================
--- linux.orig/drivers/mtd/chips/cfi_util.c
+++ linux/drivers/mtd/chips/cfi_util.c
@@ -26,7 +26,7 @@
 void cfi_udelay(int us)
 {
 	if (us >= 1000) {
-		msleep((us+999)/1000);
+		msleep(DIV_ROUND_UP(us, 1000));
 	} else {
 		udelay(us);
 		cond_resched();

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: cfi_util: use DIV_ROUND_UP() in cfi_udelay()

[Geert Uytterhoeven]

Hi Sergei,

On Tue, Sep 17, 2019 at 9:28 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> cfi_udelay() open-codes DIV_ROUND_UP(), violating the kernel coding style

Perhaps "violating" sounds a bit too harsh?

> while  doing this...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: cfi_util: use DIV_ROUND_UP() in cfi_udelay()

[Sergei Shtylyov]

Hello!

On 09/17/2019 10:43 PM, Geert Uytterhoeven wrote:

>> cfi_udelay() open-codes DIV_ROUND_UP(), violating the kernel coding style
> 
> Perhaps "violating" sounds a bit too harsh?

   Hm, indeed, scripts/checkpatch.pl doesn't complain on this line. Do you have
other ideas how to call this? Or just omit this?

>> while  doing this...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

   TY!

> Gr{oetje,eeting}s,
> 
>                         Geert

MBR, Sergei

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: cfi_util: use DIV_ROUND_UP() in cfi_udelay()

[Geert Uytterhoeven]

Hi Sergei,

On Tue, Sep 17, 2019 at 9:53 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 09/17/2019 10:43 PM, Geert Uytterhoeven wrote:
> >> cfi_udelay() open-codes DIV_ROUND_UP(), violating the kernel coding style
> >
> > Perhaps "violating" sounds a bit too harsh?
>
>    Hm, indeed, scripts/checkpatch.pl doesn't complain on this line. Do you have
> other ideas how to call this? Or just omit this?

Just "use the existing helper, instead of open-coding the same operation"?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: cfi_util: use DIV_ROUND_UP() in cfi_udelay()

[Vignesh Raghavendra]

Hi Sergei,

On 18/09/19 2:53 AM, Geert Uytterhoeven wrote:
> Hi Sergei,
> 
> On Tue, Sep 17, 2019 at 9:53 PM Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
>> On 09/17/2019 10:43 PM, Geert Uytterhoeven wrote:
>>>> cfi_udelay() open-codes DIV_ROUND_UP(), violating the kernel coding style
>>>
>>> Perhaps "violating" sounds a bit too harsh?
>>
>>    Hm, indeed, scripts/checkpatch.pl doesn't complain on this line. Do you have
>> other ideas how to call this? Or just omit this?
> 

scripts/checkpatch.pl --strict will complain about this.

> Just "use the existing helper, instead of open-coding the same operation"?

I agree with Geert. This driver file predates checkpatch and therefore
does not follow all kernel coding styles. But its good to replace
open-coding with available helper macro.

Also, please don't post new patches in reply to an existing thread. This
patch appears in-reply-to ("mtd: devices: m25p80: Use the spi-mem dirmap
API") which is unrelated to current patch.


-- 
Regards
Vignesh

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: cfi_util: use DIV_ROUND_UP() in cfi_udelay()

[Sergei Shtylyov]

On 09/18/2019 08:45 AM, Vignesh Raghavendra wrote:

>>>>> cfi_udelay() open-codes DIV_ROUND_UP(), violating the kernel coding style
>>>>
>>>> Perhaps "violating" sounds a bit too harsh?
>>>
>>>    Hm, indeed, scripts/checkpatch.pl doesn't complain on this line. Do you have
>>> other ideas how to call this? Or just omit this?
>>
> 
> scripts/checkpatch.pl --strict will complain about this.
> 
>> Just "use the existing helper, instead of open-coding the same operation"?
> 
> I agree with Geert. This driver file predates checkpatch and therefore
> does not follow all kernel coding styles. But its good to replace
> open-coding with available helper macro.
> 
> Also, please don't post new patches in reply to an existing thread. This

   I don't think I did this. It wasn't intended anyway, sorry...

> patch appears in-reply-to ("mtd: devices: m25p80: Use the spi-mem dirmap
> API") which is unrelated to current patch.

    Indeed.

MBR, Sergei

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH] mtd: spi-nor: use spi-mem dirmap API

[Sergei Shtylyov]

Make use of the spi-mem direct mapping API to let advanced controllers
optimize read/write operations when they support direct mapping.

Based on the original patch by Boris Brezillon <boris.brezillon@bootlin.com>.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
This patch is against the 'spi-nor/next' branch of the MTD 'linux.git' repo.

 drivers/mtd/spi-nor/spi-nor.c |   79 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spi-nor.h   |    5 ++
 2 files changed, 84 insertions(+)

Index: linux/drivers/mtd/spi-nor/spi-nor.c
===================================================================
--- linux.orig/drivers/mtd/spi-nor/spi-nor.c
+++ linux/drivers/mtd/spi-nor/spi-nor.c
@@ -2562,6 +2562,14 @@ static int spi_nor_read(struct mtd_info
 	if (ret)
 		return ret;
 
+	if (nor->dirmap.rdesc) {
+		ret = spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf);
+		if (ret < 0)
+			goto read_err;
+		*retlen += ret;
+		goto done;
+	}
+
 	while (len) {
 		loff_t addr = from;
 
@@ -2582,6 +2590,7 @@ static int spi_nor_read(struct mtd_info
 		from += ret;
 		len -= ret;
 	}
+done:
 	ret = 0;
 
 read_err:
@@ -2686,6 +2695,14 @@ static int spi_nor_write(struct mtd_info
 	if (ret)
 		return ret;
 
+	if (nor->dirmap.wdesc) {
+		ret = spi_mem_dirmap_write(nor->dirmap.wdesc, to, len, buf);
+		if (ret < 0)
+			goto write_err;
+		*retlen += ret;
+		goto done;
+	}
+
 	for (i = 0; i < len; ) {
 		ssize_t written;
 		loff_t addr = to + i;
@@ -2723,6 +2740,8 @@ static int spi_nor_write(struct mtd_info
 		*retlen += written;
 		i += written;
 	}
+done:
+	ret = 0;
 
 write_err:
 	spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_WRITE);
@@ -4991,6 +5010,58 @@ int spi_nor_scan(struct spi_nor *nor, co
 }
 EXPORT_SYMBOL_GPL(spi_nor_scan);
 
+static int spi_nor_create_read_dirmap(struct spi_nor *nor)
+{
+	struct spi_mem_dirmap_info info = {
+		.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
+				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 1),
+				      SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
+				      SPI_MEM_OP_DATA_IN(0, NULL, 1)),
+		.offset = 0,
+		.length = nor->mtd.size,
+	};
+	struct spi_mem_op *op = &info.op_tmpl;
+
+	/* get transfer protocols. */
+	op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
+	op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
+	op->dummy.buswidth = op->addr.buswidth;
+	op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
+
+	/* convert the dummy cycles to the number of bytes */
+	op->dummy.nbytes = (nor->read_dummy * op->dummy.buswidth) / 8;
+
+	nor->dirmap.rdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem,
+						       &info);
+	return PTR_ERR_OR_ZERO(nor->dirmap.rdesc);
+}
+
+static int spi_nor_create_write_dirmap(struct spi_nor *nor)
+{
+	struct spi_mem_dirmap_info info = {
+		.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
+				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 1),
+				      SPI_MEM_OP_NO_DUMMY,
+				      SPI_MEM_OP_DATA_OUT(0, NULL, 1)),
+		.offset = 0,
+		.length = nor->mtd.size,
+	};
+	struct spi_mem_op *op = &info.op_tmpl;
+
+	/* get transfer protocols. */
+	op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
+	op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
+	op->dummy.buswidth = op->addr.buswidth;
+	op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
+
+	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
+		op->addr.nbytes = 0;
+
+	nor->dirmap.wdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem,
+						       &info);
+	return PTR_ERR_OR_ZERO(nor->dirmap.wdesc);
+}
+
 static int spi_nor_probe(struct spi_mem *spimem)
 {
 	struct spi_device *spi = spimem->spi;
@@ -5052,6 +5123,14 @@ static int spi_nor_probe(struct spi_mem
 			return -ENOMEM;
 	}
 
+	ret = spi_nor_create_read_dirmap(nor);
+	if (ret)
+		return ret;
+
+	ret = spi_nor_create_write_dirmap(nor);
+	if (ret)
+		return ret;
+
 	return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
 				   data ? data->nr_parts : 0);
 }
Index: linux/include/linux/mtd/spi-nor.h
===================================================================
--- linux.orig/include/linux/mtd/spi-nor.h
+++ linux/include/linux/mtd/spi-nor.h
@@ -611,6 +611,11 @@ struct spi_nor {
 	int (*clear_sr_bp)(struct spi_nor *nor);
 	struct spi_nor_flash_parameter params;
 
+	struct {
+		struct spi_mem_dirmap_desc *rdesc;
+		struct spi_mem_dirmap_desc *wdesc;
+	} dirmap;
+
 	void *priv;
 };
 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: use spi-mem dirmap API

[Boris Brezillon]

On Fri, 25 Oct 2019 23:26:42 +0300
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:

> Make use of the spi-mem direct mapping API to let advanced controllers
> optimize read/write operations when they support direct mapping.
> 
> Based on the original patch by Boris Brezillon <boris.brezillon@bootlin.com>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> This patch is against the 'spi-nor/next' branch of the MTD 'linux.git' repo.
> 
>  drivers/mtd/spi-nor/spi-nor.c |   79 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spi-nor.h   |    5 ++
>  2 files changed, 84 insertions(+)
> 
> Index: linux/drivers/mtd/spi-nor/spi-nor.c
> ===================================================================
> --- linux.orig/drivers/mtd/spi-nor/spi-nor.c
> +++ linux/drivers/mtd/spi-nor/spi-nor.c
> @@ -2562,6 +2562,14 @@ static int spi_nor_read(struct mtd_info
>  	if (ret)
>  		return ret;
>  
> +	if (nor->dirmap.rdesc) {
> +		ret = spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf);

This spi_mem_dirmap_read() call should be moved to
spi_nor_spimem_read_data().

> +		if (ret < 0)
> +			goto read_err;
> +		*retlen += ret;
> +		goto done;
> +	}
> +
>  	while (len) {
>  		loff_t addr = from;
>  
> @@ -2582,6 +2590,7 @@ static int spi_nor_read(struct mtd_info
>  		from += ret;
>  		len -= ret;
>  	}
> +done:
>  	ret = 0;
>  
>  read_err:
> @@ -2686,6 +2695,14 @@ static int spi_nor_write(struct mtd_info
>  	if (ret)
>  		return ret;
>  
> +	if (nor->dirmap.wdesc) {
> +		ret = spi_mem_dirmap_write(nor->dirmap.wdesc, to, len, buf);

Same here, this should be moved to spi_nor_spimem_write_data(). BTW, I
wonder how this can work since write_enable() is called in the below for
loop (which you skip). Is your SPI controller sending the WE cmd
automatically?

> +		if (ret < 0)
> +			goto write_err;
> +		*retlen += ret;
> +		goto done;
> +	}
> +
>  	for (i = 0; i < len; ) {
>  		ssize_t written;
>  		loff_t addr = to + i;
> @@ -2723,6 +2740,8 @@ static int spi_nor_write(struct mtd_info
>  		*retlen += written;
>  		i += written;
>  	}
> +done:
> +	ret = 0;
>  
>  write_err:
>  	spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_WRITE);
> @@ -4991,6 +5010,58 @@ int spi_nor_scan(struct spi_nor *nor, co
>  }
>  EXPORT_SYMBOL_GPL(spi_nor_scan);
>  


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: use spi-mem dirmap API

[Sergei Shtylyov]

Hello!

On 10/26/2019 10:36 AM, Boris Brezillon wrote:

>> Make use of the spi-mem direct mapping API to let advanced controllers
>> optimize read/write operations when they support direct mapping.
>>
>> Based on the original patch by Boris Brezillon <boris.brezillon@bootlin.com>.
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> This patch is against the 'spi-nor/next' branch of the MTD 'linux.git' repo.
>>
>>  drivers/mtd/spi-nor/spi-nor.c |   79 ++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/mtd/spi-nor.h   |    5 ++
>>  2 files changed, 84 insertions(+)
>>
>> Index: linux/drivers/mtd/spi-nor/spi-nor.c
>> ===================================================================
>> --- linux.orig/drivers/mtd/spi-nor/spi-nor.c
>> +++ linux/drivers/mtd/spi-nor/spi-nor.c
>> @@ -2562,6 +2562,14 @@ static int spi_nor_read(struct mtd_info
>>  	if (ret)
>>  		return ret;
>>  
>> +	if (nor->dirmap.rdesc) {
>> +		ret = spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf);
> 
> This spi_mem_dirmap_read() call should be moved to
> spi_nor_spimem_read_data().
> 
>> +		if (ret < 0)
>> +			goto read_err;
>> +		*retlen += ret;
>> +		goto done;
>> +	}
>> +
>>  	while (len) {
>>  		loff_t addr = from;
>>  
>> @@ -2582,6 +2590,7 @@ static int spi_nor_read(struct mtd_info
>>  		from += ret;
>>  		len -= ret;
>>  	}
>> +done:
>>  	ret = 0;
>>  
>>  read_err:
>> @@ -2686,6 +2695,14 @@ static int spi_nor_write(struct mtd_info
>>  	if (ret)
>>  		return ret;
>>  
>> +	if (nor->dirmap.wdesc) {
>> +		ret = spi_mem_dirmap_write(nor->dirmap.wdesc, to, len, buf);
> 
> Same here, this should be moved to spi_nor_spimem_write_data(). BTW, I
> wonder how this can work since write_enable() is called in the below for
> loop (which you skip). Is your SPI controller sending the WE cmd
> automatically?

   Probably not, it just doesn't have a write dirmap, only read, so this part
didn't get tested... :-)

>> +		if (ret < 0)
>> +			goto write_err;
>> +		*retlen += ret;
>> +		goto done;
>> +	}
>> +
>>  	for (i = 0; i < len; ) {
>>  		ssize_t written;
>>  		loff_t addr = to + i;
[...]

MBR, Sergei

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2] mtd: spi-nor: use spi-mem dirmap API

[Sergei Shtylyov]

Make use of the spi-mem direct mapping API to let advanced controllers
optimize read/write operations when they support direct mapping.

Based on the original patch by Boris Brezillon <boris.brezillon@bootlin.com>.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Changes in version 2:
- moved the spi_mem_dirmap_{read|write}() calls from spi_nor_{read|write}() to
  spi_nor_spimem_{read|write}_data().

 drivers/mtd/spi-nor/spi-nor.c |  125 +++++++++++++++++++++++++++++++++---------
 include/linux/mtd/spi-nor.h   |    5 +
 2 files changed, 104 insertions(+), 26 deletions(-)

Index: linux/drivers/mtd/spi-nor/spi-nor.c
===================================================================
--- linux.orig/drivers/mtd/spi-nor/spi-nor.c
+++ linux/drivers/mtd/spi-nor/spi-nor.c
@@ -305,22 +305,28 @@ static ssize_t spi_nor_spimem_xfer_data(
 static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from,
 					size_t len, u8 *buf)
 {
-	struct spi_mem_op op =
-		SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
-			   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
-			   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
-			   SPI_MEM_OP_DATA_IN(len, buf, 1));
-
-	/* get transfer protocols. */
-	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
-	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
-	op.dummy.buswidth = op.addr.buswidth;
-	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
+	if (!nor->dirmap.rdesc) {
+		struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
+				   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
+				   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
+				   SPI_MEM_OP_DATA_IN(len, buf, 1));
+
+		/* get transfer protocols. */
+		op.cmd.buswidth =
+			spi_nor_get_protocol_inst_nbits(nor->read_proto);
+		op.addr.buswidth =
+			spi_nor_get_protocol_addr_nbits(nor->read_proto);
+		op.dummy.buswidth = op.addr.buswidth;
+		op.data.buswidth =
+			spi_nor_get_protocol_data_nbits(nor->read_proto);
 
-	/* convert the dummy cycles to the number of bytes */
-	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
+		/* convert the dummy cycles to the number of bytes */
+		op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
 
-	return spi_nor_spimem_xfer_data(nor, &op);
+		return spi_nor_spimem_xfer_data(nor, &op);
+	}
+	return spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf);
 }
 
 /**
@@ -354,20 +360,27 @@ static ssize_t spi_nor_read_data(struct
 static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to,
 					 size_t len, const u8 *buf)
 {
-	struct spi_mem_op op =
-		SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
-			   SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
-			   SPI_MEM_OP_NO_DUMMY,
-			   SPI_MEM_OP_DATA_OUT(len, buf, 1));
-
-	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
-	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
-	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
+	if (!nor->dirmap.wdesc) {
+		struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
+				   SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
+				   SPI_MEM_OP_NO_DUMMY,
+				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
 
-	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
-		op.addr.nbytes = 0;
+		op.cmd.buswidth =
+			spi_nor_get_protocol_inst_nbits(nor->write_proto);
+		op.addr.buswidth =
+			spi_nor_get_protocol_addr_nbits(nor->write_proto);
+		op.data.buswidth =
+			spi_nor_get_protocol_data_nbits(nor->write_proto);
+
+		if (nor->program_opcode == SPINOR_OP_AAI_WP &&
+		    nor->sst_write_second)
+			op.addr.nbytes = 0;
 
-	return spi_nor_spimem_xfer_data(nor, &op);
+		return spi_nor_spimem_xfer_data(nor, &op);
+	}
+	return spi_mem_dirmap_write(nor->dirmap.wdesc, to, len, buf);
 }
 
 /**
@@ -4979,6 +4992,58 @@ int spi_nor_scan(struct spi_nor *nor, co
 }
 EXPORT_SYMBOL_GPL(spi_nor_scan);
 
+static int spi_nor_create_read_dirmap(struct spi_nor *nor)
+{
+	struct spi_mem_dirmap_info info = {
+		.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
+				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 1),
+				      SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
+				      SPI_MEM_OP_DATA_IN(0, NULL, 1)),
+		.offset = 0,
+		.length = nor->mtd.size,
+	};
+	struct spi_mem_op *op = &info.op_tmpl;
+
+	/* get transfer protocols. */
+	op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
+	op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
+	op->dummy.buswidth = op->addr.buswidth;
+	op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
+
+	/* convert the dummy cycles to the number of bytes */
+	op->dummy.nbytes = (nor->read_dummy * op->dummy.buswidth) / 8;
+
+	nor->dirmap.rdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem,
+						       &info);
+	return PTR_ERR_OR_ZERO(nor->dirmap.rdesc);
+}
+
+static int spi_nor_create_write_dirmap(struct spi_nor *nor)
+{
+	struct spi_mem_dirmap_info info = {
+		.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
+				      SPI_MEM_OP_ADDR(nor->addr_width, 0, 1),
+				      SPI_MEM_OP_NO_DUMMY,
+				      SPI_MEM_OP_DATA_OUT(0, NULL, 1)),
+		.offset = 0,
+		.length = nor->mtd.size,
+	};
+	struct spi_mem_op *op = &info.op_tmpl;
+
+	/* get transfer protocols. */
+	op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
+	op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
+	op->dummy.buswidth = op->addr.buswidth;
+	op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
+
+	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
+		op->addr.nbytes = 0;
+
+	nor->dirmap.wdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem,
+						       &info);
+	return PTR_ERR_OR_ZERO(nor->dirmap.wdesc);
+}
+
 static int spi_nor_probe(struct spi_mem *spimem)
 {
 	struct spi_device *spi = spimem->spi;
@@ -5040,6 +5105,14 @@ static int spi_nor_probe(struct spi_mem
 			return -ENOMEM;
 	}
 
+	ret = spi_nor_create_read_dirmap(nor);
+	if (ret)
+		return ret;
+
+	ret = spi_nor_create_write_dirmap(nor);
+	if (ret)
+		return ret;
+
 	return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
 				   data ? data->nr_parts : 0);
 }
Index: linux/include/linux/mtd/spi-nor.h
===================================================================
--- linux.orig/include/linux/mtd/spi-nor.h
+++ linux/include/linux/mtd/spi-nor.h
@@ -602,6 +602,11 @@ struct spi_nor {
 	int (*clear_sr_bp)(struct spi_nor *nor);
 	struct spi_nor_flash_parameter params;
 
+	struct {
+		struct spi_mem_dirmap_desc *rdesc;
+		struct spi_mem_dirmap_desc *wdesc;
+	} dirmap;
+
 	void *priv;
 };
 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: spi-nor: use spi-mem dirmap API

[Sergei Shtylyov]

On 11/07/2019 11:49 PM, Sergei Shtylyov wrote:

> Make use of the spi-mem direct mapping API to let advanced controllers
> optimize read/write operations when they support direct mapping.
> 
> Based on the original patch by Boris Brezillon <boris.brezillon@bootlin.com>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---

   Oope, forgot to mention the base. Was generated again mtd/fixes by mistake,
should still apply to spi-nor/next with only small offsets. 

> Changes in version 2:
> - moved the spi_mem_dirmap_{read|write}() calls from spi_nor_{read|write}() to
>   spi_nor_spimem_{read|write}_data().
> 
>  drivers/mtd/spi-nor/spi-nor.c |  125 +++++++++++++++++++++++++++++++++---------
>  include/linux/mtd/spi-nor.h   |    5 +
>  2 files changed, 104 insertions(+), 26 deletions(-)
[...]

MBR, Sergei

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: spi-nor: use spi-mem dirmap API

[Sergei Shtylyov]

On 11/07/2019 11:49 PM, Sergei Shtylyov wrote:

> Make use of the spi-mem direct mapping API to let advanced controllers
> optimize read/write operations when they support direct mapping.
> 
> Based on the original patch by Boris Brezillon <boris.brezillon@bootlin.com>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---

   Oops, forgot to mention the base. Was generated again mtd/fixes by mistake,
should still apply to spi-nor/next with only small offsets. 

> Changes in version 2:
> - moved the spi_mem_dirmap_{read|write}() calls from spi_nor_{read|write}() to
>   spi_nor_spimem_{read|write}_data().
> 
>  drivers/mtd/spi-nor/spi-nor.c |  125 +++++++++++++++++++++++++++++++++---------
>  include/linux/mtd/spi-nor.h   |    5 +
>  2 files changed, 104 insertions(+), 26 deletions(-)
[...]

MBR, Sergei

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: spi-nor: use spi-mem dirmap API

[Boris Brezillon]

On Thu, 7 Nov 2019 23:49:12 +0300
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:

> Make use of the spi-mem direct mapping API to let advanced controllers
> optimize read/write operations when they support direct mapping.
> 
> Based on the original patch by Boris Brezillon <boris.brezillon@bootlin.com>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> Changes in version 2:
> - moved the spi_mem_dirmap_{read|write}() calls from spi_nor_{read|write}() to
>   spi_nor_spimem_{read|write}_data().
> 
>  drivers/mtd/spi-nor/spi-nor.c |  125 +++++++++++++++++++++++++++++++++---------
>  include/linux/mtd/spi-nor.h   |    5 +
>  2 files changed, 104 insertions(+), 26 deletions(-)
> 
> Index: linux/drivers/mtd/spi-nor/spi-nor.c
> ===================================================================
> --- linux.orig/drivers/mtd/spi-nor/spi-nor.c
> +++ linux/drivers/mtd/spi-nor/spi-nor.c
> @@ -305,22 +305,28 @@ static ssize_t spi_nor_spimem_xfer_data(
>  static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from,
>  					size_t len, u8 *buf)
>  {
> -	struct spi_mem_op op =
> -		SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
> -			   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
> -			   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
> -			   SPI_MEM_OP_DATA_IN(len, buf, 1));
> -
> -	/* get transfer protocols. */
> -	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
> -	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
> -	op.dummy.buswidth = op.addr.buswidth;
> -	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
> +	if (!nor->dirmap.rdesc) {
> +		struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
> +				   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
> +				   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
> +				   SPI_MEM_OP_DATA_IN(len, buf, 1));
> +
> +		/* get transfer protocols. */
> +		op.cmd.buswidth =
> +			spi_nor_get_protocol_inst_nbits(nor->read_proto);
> +		op.addr.buswidth =
> +			spi_nor_get_protocol_addr_nbits(nor->read_proto);
> +		op.dummy.buswidth = op.addr.buswidth;
> +		op.data.buswidth =
> +			spi_nor_get_protocol_data_nbits(nor->read_proto);
>  
> -	/* convert the dummy cycles to the number of bytes */
> -	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
> +		/* convert the dummy cycles to the number of bytes */
> +		op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
>  
> -	return spi_nor_spimem_xfer_data(nor, &op);
> +		return spi_nor_spimem_xfer_data(nor, &op);
> +	}
> +	return spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf);

Can we put the spi_mem_dirmap_read() in the if() branch instead of
having an extra level of indentation for the most complex block.

	if (nor->dirmap.rdesc)
		return spi_mem_dirmap_read(nor->dirmap.rdesc, from,
					   len, buf);

	...

(same comment applies to the write path BTW).

With this addressed, you can add

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

Thanks,

Boris


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: spi-nor: use spi-mem dirmap API

[Sergei Shtylyov]

Hello!

On 11/09/2019 10:35 PM, Boris Brezillon wrote:

>> Make use of the spi-mem direct mapping API to let advanced controllers
>> optimize read/write operations when they support direct mapping.
>>
>> Based on the original patch by Boris Brezillon <boris.brezillon@bootlin.com>.
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> Changes in version 2:
>> - moved the spi_mem_dirmap_{read|write}() calls from spi_nor_{read|write}() to
>>   spi_nor_spimem_{read|write}_data().
>>
>>  drivers/mtd/spi-nor/spi-nor.c |  125 +++++++++++++++++++++++++++++++++---------
>>  include/linux/mtd/spi-nor.h   |    5 +
>>  2 files changed, 104 insertions(+), 26 deletions(-)
>>
>> Index: linux/drivers/mtd/spi-nor/spi-nor.c
>> ===================================================================
>> --- linux.orig/drivers/mtd/spi-nor/spi-nor.c
>> +++ linux/drivers/mtd/spi-nor/spi-nor.c
>> @@ -305,22 +305,28 @@ static ssize_t spi_nor_spimem_xfer_data(
>>  static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from,
>>  					size_t len, u8 *buf)
>>  {
>> -	struct spi_mem_op op =
>> -		SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
>> -			   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
>> -			   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
>> -			   SPI_MEM_OP_DATA_IN(len, buf, 1));
>> -
>> -	/* get transfer protocols. */
>> -	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
>> -	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
>> -	op.dummy.buswidth = op.addr.buswidth;
>> -	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
>> +	if (!nor->dirmap.rdesc) {
>> +		struct spi_mem_op op =
>> +			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
>> +				   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
>> +				   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
>> +				   SPI_MEM_OP_DATA_IN(len, buf, 1));
>> +
>> +		/* get transfer protocols. */
>> +		op.cmd.buswidth =
>> +			spi_nor_get_protocol_inst_nbits(nor->read_proto);
>> +		op.addr.buswidth =
>> +			spi_nor_get_protocol_addr_nbits(nor->read_proto);
>> +		op.dummy.buswidth = op.addr.buswidth;
>> +		op.data.buswidth =
>> +			spi_nor_get_protocol_data_nbits(nor->read_proto);
>>  
>> -	/* convert the dummy cycles to the number of bytes */
>> -	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
>> +		/* convert the dummy cycles to the number of bytes */
>> +		op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
>>  
>> -	return spi_nor_spimem_xfer_data(nor, &op);
>> +		return spi_nor_spimem_xfer_data(nor, &op);
>> +	}
>> +	return spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf);
> 
> Can we put the spi_mem_dirmap_read() in the if() branch instead of
> having an extra level of indentation for the most complex block.

   Have you noticed a complex variable initializer in that block? Do you want
it to be done on the simple branch as well? 

> 	if (nor->dirmap.rdesc)
> 		return spi_mem_dirmap_read(nor->dirmap.rdesc, from,
> 					   len, buf);
> 
> 	...
> 
> (same comment applies to the write path BTW).
> 
> With this addressed, you can add
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

   Thank you. :-)

> 
> Thanks,
> 
> Boris

MBR, Sergei

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH] mtd: spi-nor: use le32_to_cpu_array()

[Sergei Shtylyov]

The driver calls le32_to_cpu() to convert the little-endian tables
to  a CPU endianness, where le32_to_cpus() should have been called.
Was going to use that one... and then discovered a whole array
converter, le32_to_cpu_array()! :-)

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
This patch is against the 'spi-nor/next' branch of the MTD 'linux.git' repo
plus couple of SPI NOR patches posted earlier...

 drivers/mtd/spi-nor/spi-nor.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Index: linux/drivers/mtd/spi-nor/spi-nor.c
===================================================================
--- linux.orig/drivers/mtd/spi-nor/spi-nor.c
+++ linux/drivers/mtd/spi-nor/spi-nor.c
@@ -3626,8 +3626,7 @@ static int spi_nor_parse_bfpt(struct spi
 		return err;
 
 	/* Fix endianness of the BFPT DWORDs. */
-	for (i = 0; i < BFPT_DWORD_MAX; i++)
-		bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
+	le32_to_cpu_array(bfpt.dwords, BFPT_DWORD_MAX);
 
 	/* Number of address bytes. */
 	switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
@@ -4085,7 +4084,7 @@ static int spi_nor_parse_smpt(struct spi
 	u32 *smpt;
 	size_t len;
 	u32 addr;
-	int i, ret;
+	int ret;
 
 	/* Read the Sector Map Parameter Table. */
 	len = smpt_header->length * sizeof(*smpt);
@@ -4099,8 +4098,7 @@ static int spi_nor_parse_smpt(struct spi
 		goto out;
 
 	/* Fix endianness of the SMPT DWORDs. */
-	for (i = 0; i < smpt_header->length; i++)
-		smpt[i] = le32_to_cpu(smpt[i]);
+	le32_to_cpu_array(smpt, smpt_header->length);
 
 	sector_map = spi_nor_get_map_in_use(nor, smpt, smpt_header->length);
 	if (IS_ERR(sector_map)) {
@@ -4193,8 +4191,7 @@ static int spi_nor_parse_4bait(struct sp
 		goto out;
 
 	/* Fix endianness of the 4BAIT DWORDs. */
-	for (i = 0; i < SFDP_4BAIT_DWORD_MAX; i++)
-		dwords[i] = le32_to_cpu(dwords[i]);
+	le32_to_cpu_array(dwords, SFDP_4BAIT_DWORD_MAX);
 
 	/*
 	 * Compute the subset of (Fast) Read commands for which the 4-byte

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: use le32_to_cpu_array()

[Tudor.Ambarus]

On Saturday, February 1, 2020 10:55:08 PM EET Sergei Shtylyov wrote:
> The driver calls le32_to_cpu() to convert the little-endian tables
> to  a CPU endianness, where le32_to_cpus() should have been called.
> Was going to use that one... and then discovered a whole array
> converter, le32_to_cpu_array()! 
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> This patch is against the 'spi-nor/next' branch of the MTD 'linux.git' repo
> plus couple of SPI NOR patches posted earlier...
> 
>  drivers/mtd/spi-nor/spi-nor.c |   11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

Applied to spi-nor/next. Thanks.

ta


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/